Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2] dynamic debug: allow printing to trace event
To:     Steven Rostedt <rostedt@goodmis.org>,
        Vincent Whitchurch <vincent.whitchurch@axis.com>
Cc:     mingo@redhat.com, kernel@axis.com, corbet@lwn.net,
        pmladek@suse.com, sergey.senozhatsky@gmail.com,
        john.ogness@linutronix.de, linux-kernel@vger.kernel.org
References: <20200814133151.7759-1-vincent.whitchurch@axis.com>
 <20200814131531.01b43c91@oasis.local.home>
From:   Jason Baron <jbaron@akamai.com>
Message-ID: <461439ab-0720-e3cc-f49f-f294fbba4129@akamai.com>
Date:   Fri, 14 Aug 2020 17:30:34 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200814131531.01b43c91@oasis.local.home>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 8/14/20 1:15 PM, Steven Rostedt wrote:
> On Fri, 14 Aug 2020 15:31:51 +0200
> Vincent Whitchurch <vincent.whitchurch@axis.com> wrote:
>> index aa9ff9e1c0b3..f599ed21ecc5 100644
>> --- a/include/linux/dynamic_debug.h
>> +++ b/include/linux/dynamic_debug.h
>> @@ -27,13 +27,16 @@ struct _ddebug {
>>  	 * writes commands to <debugfs>/dynamic_debug/control
>>  	 */
>>  #define _DPRINTK_FLAGS_NONE	0
>> -#define _DPRINTK_FLAGS_PRINT	(1<<0) /* printk() a message using the format */
>> +#define _DPRINTK_FLAGS_PRINTK	(1<<0) /* printk() a message using the format */
> 
> The above looks like a cleanup unrelated to this patch, and probably
> should be on its own.

I read it as we used to have this one thing called 'print', which really meant
printk, but now that we also have the ability to output to the trace buffer,
what does 'print' mean now? So I read it as being part of this change.

> 
>>  #define _DPRINTK_FLAGS_INCL_MODNAME	(1<<1)
>>  #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
>>  #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
>>  #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
>> +#define _DPRINTK_FLAGS_TRACE		(1<<5)	
>> +#define _DPRINTK_FLAGS_PRINT		(_DPRINTK_FLAGS_PRINTK | \
>> +					 _DPRINTK_FLAGS_TRACE)


Is _DPRINTK_FLAGS_PRINT actually used anywhere? Looks to me like
it can be removed.

This is a feature I've wanted for dynamic debug for a while. Thanks for
implementing it!

Dynamic can be enabled on the command line in order to print things early
in boot (I think ftrace can as well), I want to make sure that there are
no ordering issues here? And things wouldn't blow up if we enable printing
to the ftrace buffer early on via dyanmic debug?

Thanks,

-Jason