Re: [PATCH v4 7/8] rv/tlob: add KUnit tests for the tlob monitor

[Gabriele Monaco]

On Wed, 2026-07-08 at 23:38 +0800, wen.yang@xxxxxxxxx wrote:
> From: Wen Yang <wen.yang@xxxxxxxxx>
>

...

> +++ b/kernel/trace/rv/monitors/tlob/.kunitconfig
> @@ -0,0 +1,8 @@
> +CONFIG_FTRACE=y
> +CONFIG_HIGH_RES_TIMERS=y
> +CONFIG_KUNIT=y
> +CONFIG_MODULES=y
> +CONFIG_RV=y
> +CONFIG_RV_MON_TLOB=y
> +CONFIG_TLOB_KUNIT_TEST=y
> +CONFIG_UPROBES=y

Let's perhaps drop here what isn't a (mostly) direct dependency and make
it slimmer? Others are requirements of those anyway (and if they stop
being so, they aren't required for the test either).

  CONFIG_KUNIT=y
  CONFIG_UPROBES=y
  CONFIG_RV=y
  CONFIG_RV_MON_TLOB=y
  CONFIG_TLOB_KUNIT_TEST=y

> diff --git a/kernel/trace/rv/monitors/tlob/Kconfig
> b/kernel/trace/rv/monitors/tlob/Kconfig
> index aa43382073d2..402ef2e5c076 100644
> --- a/kernel/trace/rv/monitors/tlob/Kconfig
> +++ b/kernel/trace/rv/monitors/tlob/Kconfig
> @@ -10,3 +10,10 @@ config RV_MON_TLOB
>  	  monitor.  tlob tracks per-task elapsed wall-clock time across a
>  	  user-delimited code section and emits error_env_tlob when the
>  	  elapsed time exceeds a configurable per-invocation budget.
> +
> +config TLOB_KUNIT_TEST
> +	tristate "KUnit tests for tlob monitor" if !KUNIT_ALL_TESTS
> +	depends on RV_MON_TLOB && KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  Enable KUnit unit tests for the tlob RV monitor.
> diff --git a/kernel/trace/rv/monitors/tlob/tlob.c
> b/kernel/trace/rv/monitors/tlob/tlob.c
> index b45e84195131..a6f9c371646c 100644
> --- a/kernel/trace/rv/monitors/tlob/tlob.c
> +++ b/kernel/trace/rv/monitors/tlob/tlob.c
> @@ -708,7 +708,7 @@ static ssize_t tlob_monitor_read(struct file *file,
>   * PATH may contain ':'; the last ':' separates path from offset.
>   * Returns 0, -EINVAL, or -ERANGE.
>   */
> -static int tlob_parse_uprobe_line(char *buf, u64 *thr_out,
> +VISIBLE_IF_KUNIT int tlob_parse_uprobe_line(char *buf, u64 *thr_out,
>  				  char **path_out,
>  				  loff_t *start_out, loff_t *stop_out)
>  {
> @@ -782,6 +782,7 @@ static int tlob_parse_uprobe_line(char *buf, u64 *thr_out,
>  	*stop_out  = (loff_t)stop_val;
>  	return 0;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(tlob_parse_uprobe_line);
>  
>  /*
>   * Parse "-PATH:OFFSET_START" (ftrace uprobe_events removal convention).
> @@ -810,8 +811,9 @@ VISIBLE_IF_KUNIT int tlob_parse_remove_line(char *buf,
> char **path_out,
>  	*start_out = (loff_t)off;
>  	return 0;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(tlob_parse_remove_line);
>  
> -VISIBLE_IF_KUNIT int tlob_create_or_delete_uprobe(char *buf)
> +static int tlob_create_or_delete_uprobe(char *buf)

That's a nit, but probably all the VISIBLE_IF_KUNIT /
EXPORT_SYMBOL_IF_KUNIT should be added by this patch alone.

And definitely tlob_create_or_delete_uprobe shouldn't have it in the
earlier patch just for it to be removed here.

>  {
>  	loff_t offset_start, offset_stop;
>  	u64 threshold_ns;
> @@ -836,7 +838,6 @@ VISIBLE_IF_KUNIT int tlob_create_or_delete_uprobe(char
> *buf)
>  	mutex_unlock(&tlob_uprobe_mutex);
>  	return ret;
>  }
> -EXPORT_SYMBOL_IF_KUNIT(tlob_create_or_delete_uprobe);
>  
>  static ssize_t tlob_monitor_write(struct file *file,
>  				  const char __user *ubuf,
> diff --git a/kernel/trace/rv/monitors/tlob/tlob.h
> b/kernel/trace/rv/monitors/tlob/tlob.h
> index fceeba748c85..15bdf3b7fade 100644
> --- a/kernel/trace/rv/monitors/tlob/tlob.h
> +++ b/kernel/trace/rv/monitors/tlob/tlob.h
> @@ -145,7 +145,9 @@ int tlob_start_task(struct task_struct *task, u64
> threshold_ns);
>  int tlob_stop_task(struct task_struct *task);
>  
>  #if IS_ENABLED(CONFIG_KUNIT)

Any reason not to use IS_ENABLED(CONFIG_TLOB_KUNIT_TEST) ?

> -int tlob_create_or_delete_uprobe(char *buf);
> +int tlob_parse_uprobe_line(char *buf, u64 *thr_out, char **path_out,
> +			   loff_t *start_out, loff_t *stop_out);
> +int tlob_parse_remove_line(char *buf, char **path_out, loff_t *start_out);

Same here, those can all be exported in this patch, no need to mention
any KUNIT stuff earlier.

>  #endif /* CONFIG_KUNIT */
>  
>  #endif /* _RV_TLOB_H */
> diff --git a/kernel/trace/rv/monitors/tlob/tlob_kunit.c
> b/kernel/trace/rv/monitors/tlob/tlob_kunit.c
> new file mode 100644
> index 000000000000..7448f3fab959
> --- /dev/null
> +++ b/kernel/trace/rv/monitors/tlob/tlob_kunit.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for the tlob RV monitor.
> + *
> + */

Besides the minor nits above, tests look good.

Thanks,
Gabriele

> +#include <kunit/test.h>
> +
> +#include "tlob.h"
> +
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
> +
> +/* Valid "p PATH:START STOP threshold=NS" lines. */
> +static const char * const tlob_parse_valid[] = {
> +	"p /usr/bin/myapp:4768 4848 threshold=5000000",
> +	"p /usr/bin/myapp:0x12a0 0x12f0 threshold=10000000",
> +	"p /opt/my:app/bin:0x100 0x200 threshold=1000000",
> +};
> +
> +/* Malformed "p ..." lines that must be rejected with -EINVAL. */
> +static const char * const tlob_parse_invalid[] = {
> +	"p :0x100 0x200 threshold=5000",
> +	"p /usr/bin/myapp:0x100 threshold=5000",
> +	"p /usr/bin/myapp:-1 0x200 threshold=5000",
> +	"p /usr/bin/myapp:0x100 -1 threshold=5000000",	/* negative stop
> offset */
> +	"p /usr/bin/myapp:0x100 0x200",
> +	"p /usr/bin/myapp:0x100 0x100 threshold=5000",
> +};
> +
> +/* threshold_ns out of valid range => -ERANGE. */
> +static const char * const tlob_parse_out_of_range[] = {
> +	"p /usr/bin/myapp:0x100 0x200 threshold=0",
> +	"p /usr/bin/myapp:0x100 0x200 threshold=999",
> +	"p /usr/bin/myapp:0x100 0x200 threshold=3600000000001",
> +};
> +
> +/* Valid "-PATH:OFFSET_START" remove lines. */
> +static const char * const tlob_remove_valid[] = {
> +	"-/usr/bin/myapp:0x100",
> +	"-/opt/my:app/bin:0x200",
> +};
> +
> +/* Malformed remove lines that must be rejected with -EINVAL. */
> +static const char * const tlob_remove_invalid[] = {
> +	"-usr/bin/myapp:0x100",
> +	"-/usr/bin/myapp",
> +	"-/:0x100",
> +	"-/usr/bin/myapp:-1",	/* negative offset */
> +	"-/usr/bin/myapp:abc",
> +};
> +
> +static void tlob_parse_valid_accepted(struct kunit *test)
> +{
> +	u64 thr;
> +	char *path;
> +	loff_t start, stop;
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_parse_valid); i++) {
> +		strscpy(buf, tlob_parse_valid[i], sizeof(buf));
> +		KUNIT_EXPECT_EQ(test, tlob_parse_uprobe_line(buf, &thr,
> &path,
> +							     &start, &stop),
> 0);
> +	}
> +}
> +
> +static void tlob_parse_invalid_rejected(struct kunit *test)
> +{
> +	u64 thr;
> +	char *path;
> +	loff_t start, stop;
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_parse_invalid); i++) {
> +		strscpy(buf, tlob_parse_invalid[i], sizeof(buf));
> +		KUNIT_EXPECT_EQ(test, tlob_parse_uprobe_line(buf, &thr,
> &path,
> +							     &start, &stop),
> -EINVAL);
> +	}
> +}
> +
> +static void tlob_parse_out_of_range_rejected(struct kunit *test)
> +{
> +	u64 thr;
> +	char *path;
> +	loff_t start, stop;
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_parse_out_of_range); i++) {
> +		strscpy(buf, tlob_parse_out_of_range[i], sizeof(buf));
> +		KUNIT_EXPECT_EQ(test, tlob_parse_uprobe_line(buf, &thr,
> &path,
> +							     &start, &stop),
> -ERANGE);
> +	}
> +}
> +
> +static void tlob_remove_valid_accepted(struct kunit *test)
> +{
> +	char *path;
> +	loff_t start;
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_remove_valid); i++) {
> +		strscpy(buf, tlob_remove_valid[i], sizeof(buf));
> +		KUNIT_EXPECT_EQ(test, tlob_parse_remove_line(buf, &path,
> &start), 0);
> +	}
> +}
> +
> +static void tlob_remove_invalid_rejected(struct kunit *test)
> +{
> +	char *path;
> +	loff_t start;
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_remove_invalid); i++) {
> +		strscpy(buf, tlob_remove_invalid[i], sizeof(buf));
> +		KUNIT_EXPECT_EQ(test, tlob_parse_remove_line(buf, &path,
> &start), -EINVAL);
> +	}
> +}
> +
> +static struct kunit_case tlob_parse_cases[] = {
> +	KUNIT_CASE(tlob_parse_valid_accepted),
> +	KUNIT_CASE(tlob_parse_invalid_rejected),
> +	KUNIT_CASE(tlob_parse_out_of_range_rejected),
> +	KUNIT_CASE(tlob_remove_valid_accepted),
> +	KUNIT_CASE(tlob_remove_invalid_rejected),
> +	{}
> +};
> +
> +static struct kunit_suite tlob_parse_suite = {
> +	.name       = "tlob_parse",
> +	.test_cases = tlob_parse_cases,
> +};
> +
> +kunit_test_suite(tlob_parse_suite);
> +
> +MODULE_DESCRIPTION("KUnit tests for the tlob RV monitor");
> +MODULE_LICENSE("GPL");