Message ID | 20200925161929.1136806-9-tasleson@redhat.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
Series | Add persistent durable identifier to storage log messages | expand |
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.
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
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(+)