diff mbox series

[RFC,33/39] blktrace: add blkext tracer

Message ID 20210225070231.21136-34-chaitanya.kulkarni@wdc.com (mailing list archive)
State New, archived
Headers show
Series [RFC,01/39] blktrace_api: add new trace definitions | expand

Commit Message

Chaitanya Kulkarni Feb. 25, 2021, 7:02 a.m. UTC
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 kernel/trace/blktrace.c | 47 ++++++++++++++++++++++++++++++++++++++---
 kernel/trace/trace.h    |  1 +
 2 files changed, 45 insertions(+), 3 deletions(-)

Comments

Steven Rostedt Feb. 27, 2021, 2:49 p.m. UTC | #1
On Sat, 27 Feb 2021 19:44:40 +0800
kernel test robot <oliver.sang@intel.com> wrote:

> [   20.216017] WARNING: CPU: 0 PID: 1 at kernel/trace/trace.c:8370 create_trace_option_files (kbuild/src/consumer/kernel/trace/trace.c:8370 (discriminator 1)) 
> [   20.218480] Modules linked in:
> [   20.219395] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.11.0-09341-gc055908abe0d #1
> [   20.221182] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
> [   20.224540] EIP: create_trace_option_files (kbuild/src/consumer/kernel/trace/trace.c:8370 (discriminator 1)) 
> [ 20.225816] Code: d5 01 83 15 2c b7 08 d5 00 83 c0 01 39 c8 0f 84 c7 00 00 00 8b 14 c7 39 72 44 75 df 83 05 10 b7 08 d5 01 83 15 14 b7 08 d5 00 <0f> 0b 83 05 18 b7 08 d5 01 83 15 1c b7 08 d5 00 83 05 20 b7 08 d5

Looks to be from this:

> +static struct tracer blk_tracer_ext __read_mostly = {
> +	.name		= "blkext",
> +	.init		= blk_tracer_init,
> +	.reset		= blk_tracer_reset,
> +	.start		= blk_tracer_start,
> +	.stop		= blk_tracer_stop,
> +	.print_header	= blk_tracer_print_header,
> +	.print_line	= blk_tracer_print_line_ext,
> +	.flags		= &blk_tracer_flags,

                          ^^^

As blk_tracer already registers those flags, when it gets registered as
a tracer, and flag names can not be duplicated.

I could fix the infrastructure to detect the same set of flags being
registered by two different tracers, but in the mean time, it may still
work to use the blk_trace_flags from blk_tracer, and keep .flags NULL
here.

-- Steve


> +	.set_flag	= blk_tracer_set_flag,
> +};
> +
Chaitanya Kulkarni Feb. 27, 2021, 7:51 p.m. UTC | #2
On 2/27/21 06:49, Steven Rostedt wrote:
> On Sat, 27 Feb 2021 19:44:40 +0800
> kernel test robot <oliver.sang@intel.com> wrote:
>
>> [   20.216017] WARNING: CPU: 0 PID: 1 at kernel/trace/trace.c:8370 create_trace_option_files (kbuild/src/consumer/kernel/trace/trace.c:8370 (discriminator 1)) 
>> [   20.218480] Modules linked in:
>> [   20.219395] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.11.0-09341-gc055908abe0d #1
>> [   20.221182] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
>> [   20.224540] EIP: create_trace_option_files (kbuild/src/consumer/kernel/trace/trace.c:8370 (discriminator 1)) 
>> [ 20.225816] Code: d5 01 83 15 2c b7 08 d5 00 83 c0 01 39 c8 0f 84 c7 00 00 00 8b 14 c7 39 72 44 75 df 83 05 10 b7 08 d5 01 83 15 14 b7 08 d5 00 <0f> 0b 83 05 18 b7 08 d5 01 83 15 1c b7 08 d5 00 83 05 20 b7 08 d5
> Looks to be from this:
>
>> +static struct tracer blk_tracer_ext __read_mostly = {
>> +	.name		= "blkext",
>> +	.init		= blk_tracer_init,
>> +	.reset		= blk_tracer_reset,
>> +	.start		= blk_tracer_start,
>> +	.stop		= blk_tracer_stop,
>> +	.print_header	= blk_tracer_print_header,
>> +	.print_line	= blk_tracer_print_line_ext,
>> +	.flags		= &blk_tracer_flags,
>                           ^^^
>
> As blk_tracer already registers those flags, when it gets registered as
> a tracer, and flag names can not be duplicated.
>
> I could fix the infrastructure to detect the same set of flags being
> registered by two different tracers, but in the mean time, it may still
> work to use the blk_trace_flags from blk_tracer, and keep .flags NULL
> here.
>
> -- Steve
Thanks for the reply Steve. This is still under currently discussion and
I'm still
waiting formore people to reply on this approach, if we end up having
this as
a part of final implementation we may need to fix that.

>
>> +	.set_flag	= blk_tracer_set_flag,
>> +};
>> +
diff mbox series

Patch

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 53bba8537294..f707ebde0062 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -2490,18 +2490,42 @@  static struct tracer blk_tracer __read_mostly = {
 	.set_flag	= blk_tracer_set_flag,
 };
 
+static struct tracer blk_tracer_ext __read_mostly = {
+	.name		= "blkext",
+	.init		= blk_tracer_init,
+	.reset		= blk_tracer_reset,
+	.start		= blk_tracer_start,
+	.stop		= blk_tracer_stop,
+	.print_header	= blk_tracer_print_header,
+	.print_line	= blk_tracer_print_line_ext,
+	.flags		= &blk_tracer_flags,
+	.set_flag	= blk_tracer_set_flag,
+};
+
 static struct trace_event_functions trace_blk_event_funcs = {
 	.trace		= blk_trace_event_print,
 	.binary		= blk_trace_event_print_binary,
 };
 
+static struct trace_event_functions trace_blk_event_funcs_ext = {
+	.trace		= blk_trace_event_print_ext,
+	.binary		= blk_trace_event_print_binary_ext,
+};
+
 static struct trace_event trace_blk_event = {
 	.type		= TRACE_BLK,
 	.funcs		= &trace_blk_event_funcs,
 };
 
+static struct trace_event trace_blk_event_ext = {
+	.type		= TRACE_BLK_EXT,
+	.funcs		= &trace_blk_event_funcs_ext,
+};
+
 static int __init init_blk_tracer(void)
 {
+	int ret = 0;
+
 	if (!register_trace_event(&trace_blk_event)) {
 		pr_warn("Warning: could not register block events\n");
 		return 1;
@@ -2509,11 +2533,28 @@  static int __init init_blk_tracer(void)
 
 	if (register_tracer(&blk_tracer) != 0) {
 		pr_warn("Warning: could not register the block tracer\n");
-		unregister_trace_event(&trace_blk_event);
-		return 1;
+		goto unregister_trace_event;
 	}
 
-	return 0;
+	if (!register_trace_event(&trace_blk_event_ext)) {
+		pr_warn("Warning: could not register block events\n");
+		/* unregister blk_tracer */
+		goto unregister_trace_event;
+	}
+
+	if (register_tracer(&blk_tracer_ext) != 0) {
+		pr_warn("Warning: could not register the block tracer\n");
+		goto unregister_trace_event_ext;
+	}
+out:
+	return ret;
+
+unregister_trace_event_ext:
+	unregister_trace_event(&trace_blk_event_ext);
+unregister_trace_event:
+	unregister_trace_event(&trace_blk_event);
+	ret = 1;
+	goto out;
 }
 
 device_initcall(init_blk_tracer);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index e448d2da0b99..8bb010753a17 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -42,6 +42,7 @@  enum trace_type {
 	TRACE_GRAPH_ENT,
 	TRACE_USER_STACK,
 	TRACE_BLK,
+	TRACE_BLK_EXT,
 	TRACE_BPUTS,
 	TRACE_HWLAT,
 	TRACE_RAW_DATA,