DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EBBEB83F3D
Subject: Re: [PATCH v3] tracing: Ignore mmiotrace from kernel commandline
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, mingo@redhat.com, karolherbst@gmail.com,
        ppaalanen@gmail.com, akpm@linux-foundation.org
References: <1505111195-31942-1-git-send-email-zsun@redhat.com>
 <3cec495b-89a5-b8c6-e319-af9f969582e0@redhat.com>
 <20170911072321.0085db2c@vmware.local.home>
From: "Ziqian SUN (Zamir)" <zsun@redhat.com>
Message-ID: <3d3bc80a-b728-e176-a280-cbfa47a6aed3@redhat.com>
Date: Tue, 12 Sep 2017 20:18:35 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
In-Reply-To: <20170911072321.0085db2c@vmware.local.home>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4600
Lines: 111


On 09/11/2017 07:23 PM, Steven Rostedt wrote:
> On Mon, 11 Sep 2017 14:31:34 +0800
> "Ziqian SUN (Zamir)" <zsun@redhat.com> wrote:
> 
>> On 09/11/2017 02:26 PM, Ziqian SUN (Zamir) wrote:
>>> From: "Ziqian SUN (Zamir)" <zsun@redhat.com>
>>>
>>> The mmiotrace tracer cannot be enabled with ftrace=mmiotrace in kernel
>>> commandline. With this patch, noboot is added to the tracer struct,
>>> and when system boot with a tracer that has noboot=true, it will print
>>> out a warning message and continue booting.
>>>
>>> Signed-off-by: Ziqian SUN (Zamir) <zsun@redhat.com>
>>> --
>>> v1 -> v2 : remove unessential comment
>>> v2 -> v3 : Use tracer struct instead of a separate list to store noboot
>>> ---
>>>    kernel/trace/trace.c           | 7 +++++++
>>>    kernel/trace/trace.h           | 2 ++
>>>    kernel/trace/trace_mmiotrace.c | 1 +
>>>    3 files changed, 10 insertions(+)
>>>
>>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>>> index 5360b7a..48c474f 100644
>>> --- a/kernel/trace/trace.c
>>> +++ b/kernel/trace/trace.c
>>> @@ -5358,6 +5358,13 @@ static int tracing_set_tracer(struct trace_array *tr, const char *buf)
>>>    	if (t == tr->current_trace)
>>>    		goto out;
>>>    
>>> +	/* Some tracers won't work on kernel command line */
>>> +	if (system_state < SYSTEM_RUNNING && t->noboot) {
>>> +		pr_warn("Tracer '%s' is not allowed on command line, ignored\n",
>>> +			t->name);
>> I feel the core trace printed by WARN is not so meaningful, so I use
>> pr_warn instead.
> 
> I'm fine with pr_warn, but I'm curious to what you mean by "not so
> meaningful"? A WARN() will cause a dump stack, which usually shows up
> as a bug in systems and more likely to be seen. But if someone is
> adding this to the kernel command line and it's not working, they
> should be looking for the tracer name within the dmesg anyway.
> 

Following is the log I tried with WARN before. With WARN we have both 
the warning message, and the stack trace. The stack trace goes into 
stacing_set_tracer, but not showing mmiotrace. I mean, this is useful 
for debugging, but maybe confusing for users who is willing to report 
bugs but not a kernel geek. With pr_warn we already indicate mmiotrace 
is ignored, and is neat.

Just my cents.

[    7.134149] Starting tracer 'mmiotrace'
[    7.134151] Tracer 'mmiotrace' is not allowed on command line
[    7.134162] ------------[ cut here ]------------
[    7.134171] WARNING: CPU: 6 PID: 1 at kernel/trace/trace.c:5354 
tracing_set_tracer+0x168/0x1b0
[    7.134171] Modules linked in:
[    7.134176] CPU: 6 PID: 1 Comm: swapper/0 Not tainted 
4.13.0-1.mmiotracev3.fc26.x86_64 #1
[    7.134178] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 09/18/2013
[    7.134179] task: ffff94b7310d8000 task.stack: ffffaadc83144000
[    7.134183] RIP: 0010:tracing_set_tracer+0x168/0x1b0
[    7.134185] RSP: 0000:ffffaadc83147da0 EFLAGS: 00010286
[    7.134187] RAX: 0000000000000031 RBX: ffffffffad04e340 RCX: 
0000000000000000
[    7.134189] RDX: 0000000000000000 RSI: 0000000000000002 RDI: 
0000000000000246
[    7.134190] RBP: ffffaadc83147dc8 R08: 0000000000000031 R09: 
ffffffffad314f00
[    7.134191] R10: ffffaadc83147d30 R11: 0000000000000000 R12: 
0000000000000000
[    7.134193] R13: ffffffffacc935f3 R14: ffffffffacc935f3 R15: 
ffffffffacef46c0
[    7.134196] FS:  0000000000000000(0000) GS:ffff94b732e00000(0000) 
knlGS:0000000000000000
[    7.134197] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    7.134199] CR2: 0000000000000000 CR3: 00000005f5e09000 CR4: 
00000000000406e0
[    7.134201] Call Trace:
[    7.134210]  ? set_debug_rodata+0x17/0x17
[    7.134213]  register_tracer+0x1a1/0x1c2
[    7.134216]  ? stack_trace_init+0xb5/0xb5
[    7.134218]  init_mmio_trace+0x10/0x12
[    7.134222]  do_one_initcall+0x50/0x190
[    7.134225]  kernel_init_freeable+0x1a8/0x245
[    7.134231]  ? rest_init+0xc0/0xc0
[    7.134234]  kernel_init+0xe/0x101
[    7.134238]  ret_from_fork+0x25/0x30
[    7.134240] Code: ff e8 6d d0 ff ff 85 c0 41 89 c4 79 8e eb b7 80 bb 
9b 00 00 00 00 0f 84 26 ff ff ff 4c 89 ee 48 c7 c7 58 1e c9 ac e8 99 a0 
f8 ff <0f> ff eb 97 48 8b 35 9d 81 d7 00 ba ff ff ff ff 4c 89 ff e8 10
[    7.134279] ---[ end trace 978ff599b6e516bc ]---

> Also, I'm currently at OSS in LA and hopefully I don't lose this patch.
> If you don't see anything by next Monday from me, feel free to send me a
> reminder ping.
> 
> -- Steve
> 

-- 
Ziqian SUN (Zamir)
9F Raycom office (NAY)
Red Hat Software (Beijing) Co.,Ltd
IRC: zsun (internal and freenode)
Tel: +86 10 65627458
GPG : 1D86 6D4A 49CE 4BBD 72CF FCF5 D856 6E11 F2A0 525E