[v5,08/12] Add durable_name_printk

Ideally block related code would standardize on using dev_printk,
but dev_printk does change the user visible messages which is
questionable.  Adding this function which adds the structured
key/value durable name to the log entry.  It has the
same signature as dev_printk.  In the future, code that
is using this could easily transition to dev_printk when that
becomes workable.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/base/core.c        | 15 +++++++++++++++
 include/linux/dev_printk.h |  5 +++++
 2 files changed, 20 insertions(+)

On 9/25/20 9:19 AM, Tony Asleson wrote:
> Ideally block related code would standardize on using dev_printk,
> but dev_printk does change the user visible messages which is
> questionable.  Adding this function which adds the structured
> key/value durable name to the log entry.  It has the
> same signature as dev_printk.  In the future, code that
> is using this could easily transition to dev_printk when that
> becomes workable.
> 
> Signed-off-by: Tony Asleson <tasleson@redhat.com>
> ---
>  drivers/base/core.c        | 15 +++++++++++++++
>  include/linux/dev_printk.h |  5 +++++
>  2 files changed, 20 insertions(+)

Hi,

I suggest that these 2 new function names should be
	printk_durable_name()
and
	printk_durable_name_ratelimited()

Those names would be closer to the printk* family of
function names.  Of course, you can find exceptions to this,
like dev_printk(), but that is in the dev_*() family of
function names.


> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 72a93b041a2d..447b0ebc93af 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3975,6 +3975,21 @@ void dev_printk(const char *level, const struct device *dev,
>  }
>  EXPORT_SYMBOL(dev_printk);
>  
> +void durable_name_printk(const char *level, const struct device *dev,
> +		const char *fmt, ...)
> +{
> +	size_t dictlen;
> +	va_list args;
> +	char dict[288];
> +
> +	dictlen = dev_durable_name(dev, dict, sizeof(dict));
> +
> +	va_start(args, fmt);
> +	vprintk_emit(0, level[1] - '0', dict, dictlen, fmt, args);
> +	va_end(args);
> +}
> +EXPORT_SYMBOL(durable_name_printk);
> +
>  #define define_dev_printk_level(func, kern_level)		\
>  void func(const struct device *dev, const char *fmt, ...)	\
>  {								\
> diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
> index 3028b644b4fb..4d57b940b692 100644
> --- a/include/linux/dev_printk.h
> +++ b/include/linux/dev_printk.h
> @@ -32,6 +32,11 @@ int dev_printk_emit(int level, const struct device *dev, const char *fmt, ...);
>  __printf(3, 4) __cold
>  void dev_printk(const char *level, const struct device *dev,
>  		const char *fmt, ...);
> +
> +__printf(3, 4) __cold
> +void durable_name_printk(const char *level, const struct device *dev,
> +			const char *fmt, ...);
> +
>  __printf(2, 3) __cold
>  void _dev_emerg(const struct device *dev, const char *fmt, ...);
>  __printf(2, 3) __cold
> 

Thanks.

On 9/26/20 6:53 PM, Randy Dunlap wrote:
> I suggest that these 2 new function names should be
> 	printk_durable_name()
> and
> 	printk_durable_name_ratelimited()
> 
> Those names would be closer to the printk* family of
> function names.  Of course, you can find exceptions to this,
> like dev_printk(), but that is in the dev_*() family of
> function names.

durable_name_printk has the same argument signature as dev_printk with
it's intention that in the future it might be a candidate to get changed
to dev_printk.  The reason I'm not using dev_printk is to avoid changing
the content of the message users see.

With this clarification, do you still suggest the rename or maybe
suggest something different?

dev_id_printk
id_printk
...

I'm also thinking that maybe we should add a new function do everything
dev_printk does, but without prepending the device driver name and
device name to the message.  So we can get the metadata adds without
having the content of the message change.

Thanks

On 9/28/20 8:52 AM, Tony Asleson wrote:
> On 9/26/20 6:53 PM, Randy Dunlap wrote:
>> I suggest that these 2 new function names should be
>> 	printk_durable_name()
>> and
>> 	printk_durable_name_ratelimited()
>>
>> Those names would be closer to the printk* family of
>> function names.  Of course, you can find exceptions to this,
>> like dev_printk(), but that is in the dev_*() family of
>> function names.
> 
> durable_name_printk has the same argument signature as dev_printk with
> it's intention that in the future it might be a candidate to get changed
> to dev_printk.  The reason I'm not using dev_printk is to avoid changing
> the content of the message users see.
> 
> With this clarification, do you still suggest the rename or maybe
> suggest something different?

Since you seem to bring it up, "durable_name" is a bit long IMO.

But yes, I still prefer printk_durable_name() etc. The other order seems
backwards to me. But that's still just an opinion.


> dev_id_printk
> id_printk
> ...
> 
> I'm also thinking that maybe we should add a new function do everything
> dev_printk does, but without prepending the device driver name and
> device name to the message.  So we can get the metadata adds without
> having the content of the message change.

thanks.