| Message ID | 1553776772-15995-2-git-send-email-galpress@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RDMA/efa: Elastic Fabric Adapter (EFA) driver | expand |
On 28-Mar-19 14:39, Gal Pressman wrote: > Similarly to dev/netdev/etc printk helpers, add standard printk helpers > for the RDMA subsystem. > > Example output: > efa 0000:00:06.0 efa_0: Hello World! > efa_0: Hello World! (no parent device set) > (NULL ib_device): Hello World! (ibdev is NULL) > > Cc: Jason Baron <jbaron@akamai.com> Forgot to add Suggested-by: Jason Gunthorpe <jgg@ziepe.ca> Suggested-by: Leon Romanovsky <leon@kernel.org> > Signed-off-by: Gal Pressman <galpress@amazon.com>
On Thu, Mar 28, 2019 at 02:39:21PM +0200, Gal Pressman wrote: > Similarly to dev/netdev/etc printk helpers, add standard printk helpers > for the RDMA subsystem. > > Example output: > efa 0000:00:06.0 efa_0: Hello World! > efa_0: Hello World! (no parent device set) > (NULL ib_device): Hello World! (ibdev is NULL) > > Cc: Jason Baron <jbaron@akamai.com> > Signed-off-by: Gal Pressman <galpress@amazon.com> Thanks Gal, Overall looks very good. > --- > drivers/infiniband/core/device.c | 60 ++++++++++++++++++++++++++++++++++++++++ > include/linux/dynamic_debug.h | 11 ++++++++ > include/rdma/ib_verbs.h | 34 +++++++++++++++++++++++ > lib/dynamic_debug.c | 40 +++++++++++++++++++++++++++ > 4 files changed, 145 insertions(+) > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > index 7421ec4883fb..a300218d3f99 100644 > --- a/drivers/infiniband/core/device.c > +++ b/drivers/infiniband/core/device.c > @@ -147,6 +147,66 @@ static int ib_security_change(struct notifier_block *nb, unsigned long event, > static void ib_policy_change_task(struct work_struct *work); > static DECLARE_WORK(ib_policy_change_work, ib_policy_change_task); > > +static void __ibdev_printk(const char *level, const struct ib_device *ibdev, > + struct va_format *vaf) > +{ > + if (ibdev && ibdev->dev.parent) > + dev_printk_emit(level[1] - '0', > + ibdev->dev.parent, > + "%s %s %s: %pV", > + dev_driver_string(ibdev->dev.parent), > + dev_name(ibdev->dev.parent), > + dev_name(&ibdev->dev), > + vaf); > + else if (ibdev) > + printk("%s%s: %pV", > + level, dev_name(&ibdev->dev), vaf); > + else > + printk("%s(NULL ib_device): %pV", level, vaf); > +} > + > +void ibdev_printk(const char *level, const struct ib_device *ibdev, > + const char *format, ...) > +{ > + struct va_format vaf; > + va_list args; > + > + va_start(args, format); > + > + vaf.fmt = format; > + vaf.va = &args; > + > + __ibdev_printk(level, ibdev, &vaf); > + > + va_end(args); > +} > +EXPORT_SYMBOL(ibdev_printk); > + > +#define define_ibdev_printk_level(func, level) \ > +void func(const struct ib_device *ibdev, const char *fmt, ...) \ > +{ \ > + struct va_format vaf; \ > + va_list args; \ > + \ > + va_start(args, fmt); \ > + \ > + vaf.fmt = fmt; \ > + vaf.va = &args; \ > + \ > + __ibdev_printk(level, ibdev, &vaf); \ > + \ > + va_end(args); \ > +} \ > +EXPORT_SYMBOL(func); > + > +define_ibdev_printk_level(ibdev_emerg, KERN_EMERG); > +define_ibdev_printk_level(ibdev_alert, KERN_ALERT); > +define_ibdev_printk_level(ibdev_crit, KERN_CRIT); > +define_ibdev_printk_level(ibdev_err, KERN_ERR); > +define_ibdev_printk_level(ibdev_warn, KERN_WARNING); > +define_ibdev_printk_level(ibdev_notice, KERN_NOTICE); > +define_ibdev_printk_level(ibdev_info, KERN_INFO); > + > static struct notifier_block ibdev_lsm_nb = { > .notifier_call = ib_security_change, > }; > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > index c2be029b9b53..6c809440f319 100644 > --- a/include/linux/dynamic_debug.h > +++ b/include/linux/dynamic_debug.h > @@ -71,6 +71,13 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, > const struct net_device *dev, > const char *fmt, ...); > > +struct ib_device; > + > +extern __printf(3, 4) > +void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > + const struct ib_device *ibdev, > + const char *fmt, ...); You guarded the implementation of this function with "CONFIG_INFINIBAND", isn't the empty declaration produced compilation warnings about missing implementation? > + > #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ > static struct _ddebug __aligned(8) \ > __attribute__((section("__verbose"))) name = { \ > @@ -154,6 +161,10 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, > _dynamic_func_call(fmt, __dynamic_netdev_dbg, \ > dev, fmt, ##__VA_ARGS__) > > +#define dynamic_ibdev_dbg(dev, fmt, ...) \ > + _dynamic_func_call(fmt, __dynamic_ibdev_dbg, \ > + dev, fmt, ##__VA_ARGS__) > + > #define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ > groupsize, buf, len, ascii) \ > _dynamic_func_call_no_desc(__builtin_constant_p(prefix_str) ? prefix_str : "hexdump", \ > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 9b9e17bcc201..f796e2d44425 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -72,6 +72,40 @@ extern struct workqueue_struct *ib_wq; > extern struct workqueue_struct *ib_comp_wq; > extern struct workqueue_struct *ib_comp_unbound_wq; > > +__printf(3, 4) __cold > +void ibdev_printk(const char *level, const struct ib_device *ibdev, > + const char *format, ...); > +__printf(2, 3) __cold > +void ibdev_emerg(const struct ib_device *ibdev, const char *format, ...); > +__printf(2, 3) __cold > +void ibdev_alert(const struct ib_device *ibdev, const char *format, ...); > +__printf(2, 3) __cold > +void ibdev_crit(const struct ib_device *ibdev, const char *format, ...); > +__printf(2, 3) __cold > +void ibdev_err(const struct ib_device *ibdev, const char *format, ...); > +__printf(2, 3) __cold > +void ibdev_warn(const struct ib_device *ibdev, const char *format, ...); > +__printf(2, 3) __cold > +void ibdev_notice(const struct ib_device *ibdev, const char *format, ...); > +__printf(2, 3) __cold > +void ibdev_info(const struct ib_device *ibdev, const char *format, ...); > + > +#if defined(CONFIG_DYNAMIC_DEBUG) > +#define ibdev_dbg(__dev, format, args...) \ > +do { \ > + dynamic_ibdev_dbg(__dev, format, ##args); \ > +} while (0) > +#elif defined(DEBUG) > +#define ibdev_dbg(__dev, format, args...) \ > + ibdev_printk(KERN_DEBUG, __dev, format, ##args) > +#else > +#define ibdev_dbg(__dev, format, args...) \ > +({ \ > + if (0) \ Why isn't "empty macro" enough here and we need "if (0)"? > + ibdev_printk(KERN_DEBUG, __dev, format, ##args); \ > +}) > +#endif > + > union ib_gid { > u8 raw[16]; > struct { > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index 7bdf98c37e91..dfcf6cfa1c70 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -37,6 +37,8 @@ > #include <linux/device.h> > #include <linux/netdevice.h> > > +#include <rdma/ib_verbs.h> > + > extern struct _ddebug __start___verbose[]; > extern struct _ddebug __stop___verbose[]; > > @@ -636,6 +638,44 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg); > > #endif > > +#if IS_ENABLED(CONFIG_INFINIBAND) > + > +void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > + const struct ib_device *ibdev, const char *fmt, ...) > +{ > + struct va_format vaf; > + va_list args; > + > + BUG_ON(!descriptor); > + BUG_ON(!fmt); > + > + va_start(args, fmt); > + > + vaf.fmt = fmt; > + vaf.va = &args; > + > + if (ibdev && ibdev->dev.parent) { > + char buf[PREFIX_SIZE]; > + > + dev_printk_emit(LOGLEVEL_DEBUG, ibdev->dev.parent, > + "%s%s %s %s: %pV", > + dynamic_emit_prefix(descriptor, buf), > + dev_driver_string(ibdev->dev.parent), > + dev_name(ibdev->dev.parent), > + dev_name(&ibdev->dev), > + &vaf); > + } else if (ibdev) { > + printk(KERN_DEBUG "%s: %pV", dev_name(&ibdev->dev), &vaf); > + } else { > + printk(KERN_DEBUG "(NULL ib_device): %pV", &vaf); > + } > + > + va_end(args); > +} > +EXPORT_SYMBOL(__dynamic_ibdev_dbg); > + > +#endif > + > #define DDEBUG_STRING_SIZE 1024 > static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE]; > > -- > 2.7.4 >
On 31-Mar-19 20:23, Leon Romanovsky wrote: > On Thu, Mar 28, 2019 at 02:39:21PM +0200, Gal Pressman wrote: >> Similarly to dev/netdev/etc printk helpers, add standard printk helpers >> for the RDMA subsystem. >> >> Example output: >> efa 0000:00:06.0 efa_0: Hello World! >> efa_0: Hello World! (no parent device set) >> (NULL ib_device): Hello World! (ibdev is NULL) >> >> Cc: Jason Baron <jbaron@akamai.com> >> Signed-off-by: Gal Pressman <galpress@amazon.com> > > Thanks Gal, > > Overall looks very good. > >> --- >> drivers/infiniband/core/device.c | 60 ++++++++++++++++++++++++++++++++++++++++ >> include/linux/dynamic_debug.h | 11 ++++++++ >> include/rdma/ib_verbs.h | 34 +++++++++++++++++++++++ >> lib/dynamic_debug.c | 40 +++++++++++++++++++++++++++ >> 4 files changed, 145 insertions(+) >> >> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c >> index 7421ec4883fb..a300218d3f99 100644 >> --- a/drivers/infiniband/core/device.c >> +++ b/drivers/infiniband/core/device.c >> @@ -147,6 +147,66 @@ static int ib_security_change(struct notifier_block *nb, unsigned long event, >> static void ib_policy_change_task(struct work_struct *work); >> static DECLARE_WORK(ib_policy_change_work, ib_policy_change_task); >> >> +static void __ibdev_printk(const char *level, const struct ib_device *ibdev, >> + struct va_format *vaf) >> +{ >> + if (ibdev && ibdev->dev.parent) >> + dev_printk_emit(level[1] - '0', >> + ibdev->dev.parent, >> + "%s %s %s: %pV", >> + dev_driver_string(ibdev->dev.parent), >> + dev_name(ibdev->dev.parent), >> + dev_name(&ibdev->dev), >> + vaf); >> + else if (ibdev) >> + printk("%s%s: %pV", >> + level, dev_name(&ibdev->dev), vaf); >> + else >> + printk("%s(NULL ib_device): %pV", level, vaf); >> +} >> + >> +void ibdev_printk(const char *level, const struct ib_device *ibdev, >> + const char *format, ...) >> +{ >> + struct va_format vaf; >> + va_list args; >> + >> + va_start(args, format); >> + >> + vaf.fmt = format; >> + vaf.va = &args; >> + >> + __ibdev_printk(level, ibdev, &vaf); >> + >> + va_end(args); >> +} >> +EXPORT_SYMBOL(ibdev_printk); >> + >> +#define define_ibdev_printk_level(func, level) \ >> +void func(const struct ib_device *ibdev, const char *fmt, ...) \ >> +{ \ >> + struct va_format vaf; \ >> + va_list args; \ >> + \ >> + va_start(args, fmt); \ >> + \ >> + vaf.fmt = fmt; \ >> + vaf.va = &args; \ >> + \ >> + __ibdev_printk(level, ibdev, &vaf); \ >> + \ >> + va_end(args); \ >> +} \ >> +EXPORT_SYMBOL(func); >> + >> +define_ibdev_printk_level(ibdev_emerg, KERN_EMERG); >> +define_ibdev_printk_level(ibdev_alert, KERN_ALERT); >> +define_ibdev_printk_level(ibdev_crit, KERN_CRIT); >> +define_ibdev_printk_level(ibdev_err, KERN_ERR); >> +define_ibdev_printk_level(ibdev_warn, KERN_WARNING); >> +define_ibdev_printk_level(ibdev_notice, KERN_NOTICE); >> +define_ibdev_printk_level(ibdev_info, KERN_INFO); >> + >> static struct notifier_block ibdev_lsm_nb = { >> .notifier_call = ib_security_change, >> }; >> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h >> index c2be029b9b53..6c809440f319 100644 >> --- a/include/linux/dynamic_debug.h >> +++ b/include/linux/dynamic_debug.h >> @@ -71,6 +71,13 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, >> const struct net_device *dev, >> const char *fmt, ...); >> >> +struct ib_device; >> + >> +extern __printf(3, 4) >> +void __dynamic_ibdev_dbg(struct _ddebug *descriptor, >> + const struct ib_device *ibdev, >> + const char *fmt, ...); > > You guarded the implementation of this function with > "CONFIG_INFINIBAND", isn't the empty declaration produced > compilation warnings about missing implementation? It doesn't produce a warning, even if CONFIG_INFINIBAND is not set. > >> + >> #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ >> static struct _ddebug __aligned(8) \ >> __attribute__((section("__verbose"))) name = { \ >> @@ -154,6 +161,10 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, >> _dynamic_func_call(fmt, __dynamic_netdev_dbg, \ >> dev, fmt, ##__VA_ARGS__) >> >> +#define dynamic_ibdev_dbg(dev, fmt, ...) \ >> + _dynamic_func_call(fmt, __dynamic_ibdev_dbg, \ >> + dev, fmt, ##__VA_ARGS__) >> + >> #define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ >> groupsize, buf, len, ascii) \ >> _dynamic_func_call_no_desc(__builtin_constant_p(prefix_str) ? prefix_str : "hexdump", \ >> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >> index 9b9e17bcc201..f796e2d44425 100644 >> --- a/include/rdma/ib_verbs.h >> +++ b/include/rdma/ib_verbs.h >> @@ -72,6 +72,40 @@ extern struct workqueue_struct *ib_wq; >> extern struct workqueue_struct *ib_comp_wq; >> extern struct workqueue_struct *ib_comp_unbound_wq; >> >> +__printf(3, 4) __cold >> +void ibdev_printk(const char *level, const struct ib_device *ibdev, >> + const char *format, ...); >> +__printf(2, 3) __cold >> +void ibdev_emerg(const struct ib_device *ibdev, const char *format, ...); >> +__printf(2, 3) __cold >> +void ibdev_alert(const struct ib_device *ibdev, const char *format, ...); >> +__printf(2, 3) __cold >> +void ibdev_crit(const struct ib_device *ibdev, const char *format, ...); >> +__printf(2, 3) __cold >> +void ibdev_err(const struct ib_device *ibdev, const char *format, ...); >> +__printf(2, 3) __cold >> +void ibdev_warn(const struct ib_device *ibdev, const char *format, ...); >> +__printf(2, 3) __cold >> +void ibdev_notice(const struct ib_device *ibdev, const char *format, ...); >> +__printf(2, 3) __cold >> +void ibdev_info(const struct ib_device *ibdev, const char *format, ...); >> + >> +#if defined(CONFIG_DYNAMIC_DEBUG) >> +#define ibdev_dbg(__dev, format, args...) \ >> +do { \ >> + dynamic_ibdev_dbg(__dev, format, ##args); \ >> +} while (0) >> +#elif defined(DEBUG) >> +#define ibdev_dbg(__dev, format, args...) \ >> + ibdev_printk(KERN_DEBUG, __dev, format, ##args) >> +#else >> +#define ibdev_dbg(__dev, format, args...) \ >> +({ \ >> + if (0) \ > > Why isn't "empty macro" enough here and we need "if (0)"? I was wondering as well :). This is copied from the netdev_dbg counterpart, my best guess is some kind of compilation checks in case debug is disabled but I can't even convince myself.. > >> + ibdev_printk(KERN_DEBUG, __dev, format, ##args); \ >> +}) >> +#endif >> + >> union ib_gid { >> u8 raw[16]; >> struct { >> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c >> index 7bdf98c37e91..dfcf6cfa1c70 100644 >> --- a/lib/dynamic_debug.c >> +++ b/lib/dynamic_debug.c >> @@ -37,6 +37,8 @@ >> #include <linux/device.h> >> #include <linux/netdevice.h> >> >> +#include <rdma/ib_verbs.h> >> + >> extern struct _ddebug __start___verbose[]; >> extern struct _ddebug __stop___verbose[]; >> >> @@ -636,6 +638,44 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg); >> >> #endif >> >> +#if IS_ENABLED(CONFIG_INFINIBAND) >> + >> +void __dynamic_ibdev_dbg(struct _ddebug *descriptor, >> + const struct ib_device *ibdev, const char *fmt, ...) >> +{ >> + struct va_format vaf; >> + va_list args; >> + >> + BUG_ON(!descriptor); >> + BUG_ON(!fmt); >> + >> + va_start(args, fmt); >> + >> + vaf.fmt = fmt; >> + vaf.va = &args; >> + >> + if (ibdev && ibdev->dev.parent) { >> + char buf[PREFIX_SIZE]; >> + >> + dev_printk_emit(LOGLEVEL_DEBUG, ibdev->dev.parent, >> + "%s%s %s %s: %pV", >> + dynamic_emit_prefix(descriptor, buf), >> + dev_driver_string(ibdev->dev.parent), >> + dev_name(ibdev->dev.parent), >> + dev_name(&ibdev->dev), >> + &vaf); >> + } else if (ibdev) { >> + printk(KERN_DEBUG "%s: %pV", dev_name(&ibdev->dev), &vaf); >> + } else { >> + printk(KERN_DEBUG "(NULL ib_device): %pV", &vaf); >> + } >> + >> + va_end(args); >> +} >> +EXPORT_SYMBOL(__dynamic_ibdev_dbg); >> + >> +#endif >> + >> #define DDEBUG_STRING_SIZE 1024 >> static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE]; >> >> -- >> 2.7.4 >>
On Mon, Apr 01, 2019 at 09:53:38AM +0300, Gal Pressman wrote: > On 31-Mar-19 20:23, Leon Romanovsky wrote: > > On Thu, Mar 28, 2019 at 02:39:21PM +0200, Gal Pressman wrote: > >> Similarly to dev/netdev/etc printk helpers, add standard printk helpers > >> for the RDMA subsystem. > >> > >> Example output: > >> efa 0000:00:06.0 efa_0: Hello World! > >> efa_0: Hello World! (no parent device set) > >> (NULL ib_device): Hello World! (ibdev is NULL) > >> > >> Cc: Jason Baron <jbaron@akamai.com> > >> Signed-off-by: Gal Pressman <galpress@amazon.com> > > > > Thanks Gal, > > > > Overall looks very good. > > > >> --- > >> drivers/infiniband/core/device.c | 60 ++++++++++++++++++++++++++++++++++++++++ > >> include/linux/dynamic_debug.h | 11 ++++++++ > >> include/rdma/ib_verbs.h | 34 +++++++++++++++++++++++ > >> lib/dynamic_debug.c | 40 +++++++++++++++++++++++++++ > >> 4 files changed, 145 insertions(+) > >> > >> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > >> index 7421ec4883fb..a300218d3f99 100644 > >> --- a/drivers/infiniband/core/device.c > >> +++ b/drivers/infiniband/core/device.c > >> @@ -147,6 +147,66 @@ static int ib_security_change(struct notifier_block *nb, unsigned long event, > >> static void ib_policy_change_task(struct work_struct *work); > >> static DECLARE_WORK(ib_policy_change_work, ib_policy_change_task); > >> > >> +static void __ibdev_printk(const char *level, const struct ib_device *ibdev, > >> + struct va_format *vaf) > >> +{ > >> + if (ibdev && ibdev->dev.parent) > >> + dev_printk_emit(level[1] - '0', > >> + ibdev->dev.parent, > >> + "%s %s %s: %pV", > >> + dev_driver_string(ibdev->dev.parent), > >> + dev_name(ibdev->dev.parent), > >> + dev_name(&ibdev->dev), > >> + vaf); > >> + else if (ibdev) > >> + printk("%s%s: %pV", > >> + level, dev_name(&ibdev->dev), vaf); > >> + else > >> + printk("%s(NULL ib_device): %pV", level, vaf); > >> +} > >> + > >> +void ibdev_printk(const char *level, const struct ib_device *ibdev, > >> + const char *format, ...) > >> +{ > >> + struct va_format vaf; > >> + va_list args; > >> + > >> + va_start(args, format); > >> + > >> + vaf.fmt = format; > >> + vaf.va = &args; > >> + > >> + __ibdev_printk(level, ibdev, &vaf); > >> + > >> + va_end(args); > >> +} > >> +EXPORT_SYMBOL(ibdev_printk); > >> + > >> +#define define_ibdev_printk_level(func, level) \ > >> +void func(const struct ib_device *ibdev, const char *fmt, ...) \ > >> +{ \ > >> + struct va_format vaf; \ > >> + va_list args; \ > >> + \ > >> + va_start(args, fmt); \ > >> + \ > >> + vaf.fmt = fmt; \ > >> + vaf.va = &args; \ > >> + \ > >> + __ibdev_printk(level, ibdev, &vaf); \ > >> + \ > >> + va_end(args); \ > >> +} \ > >> +EXPORT_SYMBOL(func); > >> + > >> +define_ibdev_printk_level(ibdev_emerg, KERN_EMERG); > >> +define_ibdev_printk_level(ibdev_alert, KERN_ALERT); > >> +define_ibdev_printk_level(ibdev_crit, KERN_CRIT); > >> +define_ibdev_printk_level(ibdev_err, KERN_ERR); > >> +define_ibdev_printk_level(ibdev_warn, KERN_WARNING); > >> +define_ibdev_printk_level(ibdev_notice, KERN_NOTICE); > >> +define_ibdev_printk_level(ibdev_info, KERN_INFO); > >> + > >> static struct notifier_block ibdev_lsm_nb = { > >> .notifier_call = ib_security_change, > >> }; > >> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > >> index c2be029b9b53..6c809440f319 100644 > >> --- a/include/linux/dynamic_debug.h > >> +++ b/include/linux/dynamic_debug.h > >> @@ -71,6 +71,13 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, > >> const struct net_device *dev, > >> const char *fmt, ...); > >> > >> +struct ib_device; > >> + > >> +extern __printf(3, 4) > >> +void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > >> + const struct ib_device *ibdev, > >> + const char *fmt, ...); > > > > You guarded the implementation of this function with > > "CONFIG_INFINIBAND", isn't the empty declaration produced > > compilation warnings about missing implementation? > > It doesn't produce a warning, even if CONFIG_INFINIBAND is not set. I tested this now and you are right. Thanks, Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
On Mon, Apr 01, 2019 at 09:53:38AM +0300, Gal Pressman wrote: > >> + > >> #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ > >> static struct _ddebug __aligned(8) \ > >> __attribute__((section("__verbose"))) name = { \ > >> @@ -154,6 +161,10 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, > >> _dynamic_func_call(fmt, __dynamic_netdev_dbg, \ > >> dev, fmt, ##__VA_ARGS__) > >> > >> +#define dynamic_ibdev_dbg(dev, fmt, ...) \ > >> + _dynamic_func_call(fmt, __dynamic_ibdev_dbg, \ > >> + dev, fmt, ##__VA_ARGS__) > >> + > >> #define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ > >> groupsize, buf, len, ascii) \ > >> _dynamic_func_call_no_desc(__builtin_constant_p(prefix_str) ? prefix_str : "hexdump", \ > >> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > >> index 9b9e17bcc201..f796e2d44425 100644 > >> +++ b/include/rdma/ib_verbs.h > >> @@ -72,6 +72,40 @@ extern struct workqueue_struct *ib_wq; > >> extern struct workqueue_struct *ib_comp_wq; > >> extern struct workqueue_struct *ib_comp_unbound_wq; > >> > >> +__printf(3, 4) __cold > >> +void ibdev_printk(const char *level, const struct ib_device *ibdev, > >> + const char *format, ...); > >> +__printf(2, 3) __cold > >> +void ibdev_emerg(const struct ib_device *ibdev, const char *format, ...); > >> +__printf(2, 3) __cold > >> +void ibdev_alert(const struct ib_device *ibdev, const char *format, ...); > >> +__printf(2, 3) __cold > >> +void ibdev_crit(const struct ib_device *ibdev, const char *format, ...); > >> +__printf(2, 3) __cold > >> +void ibdev_err(const struct ib_device *ibdev, const char *format, ...); > >> +__printf(2, 3) __cold > >> +void ibdev_warn(const struct ib_device *ibdev, const char *format, ...); > >> +__printf(2, 3) __cold > >> +void ibdev_notice(const struct ib_device *ibdev, const char *format, ...); > >> +__printf(2, 3) __cold > >> +void ibdev_info(const struct ib_device *ibdev, const char *format, ...); > >> + > >> +#if defined(CONFIG_DYNAMIC_DEBUG) > >> +#define ibdev_dbg(__dev, format, args...) \ > >> +do { \ > >> + dynamic_ibdev_dbg(__dev, format, ##args); \ > >> +} while (0) This is weird too, why the while()? > >> +#elif defined(DEBUG) > >> +#define ibdev_dbg(__dev, format, args...) \ > >> + ibdev_printk(KERN_DEBUG, __dev, format, ##args) > >> +#else > >> +#define ibdev_dbg(__dev, format, args...) \ > >> +({ \ > >> + if (0) \ > > > > Why isn't "empty macro" enough here and we need "if (0)"? > > I was wondering as well :). > This is copied from the netdev_dbg counterpart, my best guess is some kind of > compilation checks in case debug is disabled but I can't even convince myself.. It is a trick to get the compiler to do a format & type check even when debug is disabled. Personally I prefer the much clearer non #define version __printf(2, 3) static inline void ibdev_dbg(const struct ib_device *dev, const char *format, ...) {} Instead of the macro tricks. Jason
> -----Original Message----- > From: Gal Pressman <galpress@amazon.com> > Sent: Thursday, March 28, 2019 7:39 AM > To: Jason Gunthorpe <jgg@ziepe.ca>; Doug Ledford <dledford@redhat.com> > Cc: Yossi Leybovich <sleybo@amazon.com>; Alexander Matushevsky > <matua@amazon.com>; Leah Shalev <shalevl@amazon.com>; Dave Goodell > <goodell@amazon.com>; Brian Barrett <bbarrett@amazon.com>; linux- > rdma@vger.kernel.org; Sean Hefty <sean.hefty@intel.com>; Dennis > Dalessandro <dennis.dalessandro@intel.com>; Leon Romanovsky > <leon@kernel.org>; Christoph Hellwig <hch@infradead.org>; Parav Pandit > <parav@mellanox.com>; Sagi Grimberg <sagi@grimberg.me>; Steve Wise > <larrystevenwise@gmail.com>; Shiraz Saleem <shiraz.saleem@intel.com>; > Gal Pressman <galpress@amazon.com>; Jason Baron <jbaron@akamai.com> > Subject: [PATCH rdma-next v4 01/12] RDMA/core: Introduce RDMA > subsystem ibdev_* print functions > > Similarly to dev/netdev/etc printk helpers, add standard printk helpers for > the RDMA subsystem. > > Example output: > efa 0000:00:06.0 efa_0: Hello World! > efa_0: Hello World! (no parent device set) (NULL ib_device): Hello World! > (ibdev is NULL) > We are working on mdev devices whose device name in kernel defined as 36 letters.. (standard uuid based) which is too long. Parent device visibility is already present at the sysfs level. Additionally with Leon's work on persistent device naming, ib device names are also going to be longer than current. Log will have more non-useful (long parent device name) information on every log entry. So we should just have ib_dev name and drop printing redundant parent device in this helper.
On Mon, Apr 01, 2019 at 05:18:16PM +0000, Parav Pandit wrote: > > > > From: Gal Pressman <galpress@amazon.com> > > Sent: Thursday, March 28, 2019 7:39 AM > > To: Jason Gunthorpe <jgg@ziepe.ca>; Doug Ledford <dledford@redhat.com> > > Cc: Yossi Leybovich <sleybo@amazon.com>; Alexander Matushevsky > > <matua@amazon.com>; Leah Shalev <shalevl@amazon.com>; Dave Goodell > > <goodell@amazon.com>; Brian Barrett <bbarrett@amazon.com>; linux- > > rdma@vger.kernel.org; Sean Hefty <sean.hefty@intel.com>; Dennis > > Dalessandro <dennis.dalessandro@intel.com>; Leon Romanovsky > > <leon@kernel.org>; Christoph Hellwig <hch@infradead.org>; Parav Pandit > > <parav@mellanox.com>; Sagi Grimberg <sagi@grimberg.me>; Steve Wise > > <larrystevenwise@gmail.com>; Shiraz Saleem <shiraz.saleem@intel.com>; > > Gal Pressman <galpress@amazon.com>; Jason Baron <jbaron@akamai.com> > > Subject: [PATCH rdma-next v4 01/12] RDMA/core: Introduce RDMA > > subsystem ibdev_* print functions > > > > Similarly to dev/netdev/etc printk helpers, add standard printk helpers for > > the RDMA subsystem. > > > > Example output: > > efa 0000:00:06.0 efa_0: Hello World! > > efa_0: Hello World! (no parent device set) (NULL ib_device): Hello World! > > (ibdev is NULL) > > > We are working on mdev devices whose device name in kernel defined > as 36 letters.. (standard uuid based) which is too long. Well, then you have a problem in netdev, so whatever is solved in netdev we should solve it here. Otherwise this should continue to copy netdev. I continue to think naming devices with random GUIDs is a really dumb thing to do. Jason
On Mon, Apr 01, 2019 at 02:47:42PM -0300, Jason Gunthorpe wrote: > On Mon, Apr 01, 2019 at 05:18:16PM +0000, Parav Pandit wrote: > > > > > > > From: Gal Pressman <galpress@amazon.com> > > > Sent: Thursday, March 28, 2019 7:39 AM > > > To: Jason Gunthorpe <jgg@ziepe.ca>; Doug Ledford <dledford@redhat.com> > > > Cc: Yossi Leybovich <sleybo@amazon.com>; Alexander Matushevsky > > > <matua@amazon.com>; Leah Shalev <shalevl@amazon.com>; Dave Goodell > > > <goodell@amazon.com>; Brian Barrett <bbarrett@amazon.com>; linux- > > > rdma@vger.kernel.org; Sean Hefty <sean.hefty@intel.com>; Dennis > > > Dalessandro <dennis.dalessandro@intel.com>; Leon Romanovsky > > > <leon@kernel.org>; Christoph Hellwig <hch@infradead.org>; Parav Pandit > > > <parav@mellanox.com>; Sagi Grimberg <sagi@grimberg.me>; Steve Wise > > > <larrystevenwise@gmail.com>; Shiraz Saleem <shiraz.saleem@intel.com>; > > > Gal Pressman <galpress@amazon.com>; Jason Baron <jbaron@akamai.com> > > > Subject: [PATCH rdma-next v4 01/12] RDMA/core: Introduce RDMA > > > subsystem ibdev_* print functions > > > > > > Similarly to dev/netdev/etc printk helpers, add standard printk helpers for > > > the RDMA subsystem. > > > > > > Example output: > > > efa 0000:00:06.0 efa_0: Hello World! > > > efa_0: Hello World! (no parent device set) (NULL ib_device): Hello World! > > > (ibdev is NULL) > > > > > > We are working on mdev devices whose device name in kernel defined > > as 36 letters.. (standard uuid based) which is too long. > > Well, then you have a problem in netdev, so whatever is solved in > netdev we should solve it here. Otherwise this should continue to copy > netdev. > > I continue to think naming devices with random GUIDs is a really dumb > thing to do. You are already doing it but in small scale by assigning some random indexes to ib_device name, e.g. in efa_X and efa_X+1, X and X+1 are GUIDs which mean nothing. Thanks > > Jason
On Mon, Apr 01, 2019 at 09:25:44PM +0300, Leon Romanovsky wrote: > On Mon, Apr 01, 2019 at 02:47:42PM -0300, Jason Gunthorpe wrote: > > On Mon, Apr 01, 2019 at 05:18:16PM +0000, Parav Pandit wrote: > > > > > > > > > > From: Gal Pressman <galpress@amazon.com> > > > > Sent: Thursday, March 28, 2019 7:39 AM > > > > To: Jason Gunthorpe <jgg@ziepe.ca>; Doug Ledford <dledford@redhat.com> > > > > Cc: Yossi Leybovich <sleybo@amazon.com>; Alexander Matushevsky > > > > <matua@amazon.com>; Leah Shalev <shalevl@amazon.com>; Dave Goodell > > > > <goodell@amazon.com>; Brian Barrett <bbarrett@amazon.com>; linux- > > > > rdma@vger.kernel.org; Sean Hefty <sean.hefty@intel.com>; Dennis > > > > Dalessandro <dennis.dalessandro@intel.com>; Leon Romanovsky > > > > <leon@kernel.org>; Christoph Hellwig <hch@infradead.org>; Parav Pandit > > > > <parav@mellanox.com>; Sagi Grimberg <sagi@grimberg.me>; Steve Wise > > > > <larrystevenwise@gmail.com>; Shiraz Saleem <shiraz.saleem@intel.com>; > > > > Gal Pressman <galpress@amazon.com>; Jason Baron <jbaron@akamai.com> > > > > Subject: [PATCH rdma-next v4 01/12] RDMA/core: Introduce RDMA > > > > subsystem ibdev_* print functions > > > > > > > > Similarly to dev/netdev/etc printk helpers, add standard printk helpers for > > > > the RDMA subsystem. > > > > > > > > Example output: > > > > efa 0000:00:06.0 efa_0: Hello World! > > > > efa_0: Hello World! (no parent device set) (NULL ib_device): Hello World! > > > > (ibdev is NULL) > > > > > > > > > We are working on mdev devices whose device name in kernel defined > > > as 36 letters.. (standard uuid based) which is too long. > > > > Well, then you have a problem in netdev, so whatever is solved in > > netdev we should solve it here. Otherwise this should continue to copy > > netdev. > > > > I continue to think naming devices with random GUIDs is a really dumb > > thing to do. > > You are already doing it but in small scale by assigning some random > indexes to ib_device name, e.g. in efa_X and efa_X+1, X and X+1 are > GUIDs which mean nothing. They are not a GUID because they are not 'global'. The GUID's this mdev thing wants to use are actualy GUIDs and have enough characters to make them properly global if randomly sourced - which nothing actually needs or wants.. Jason
On 01-Apr-19 18:56, Jason Gunthorpe wrote: > On Mon, Apr 01, 2019 at 09:53:38AM +0300, Gal Pressman wrote: >>>> + >>>> #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ >>>> static struct _ddebug __aligned(8) \ >>>> __attribute__((section("__verbose"))) name = { \ >>>> @@ -154,6 +161,10 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, >>>> _dynamic_func_call(fmt, __dynamic_netdev_dbg, \ >>>> dev, fmt, ##__VA_ARGS__) >>>> >>>> +#define dynamic_ibdev_dbg(dev, fmt, ...) \ >>>> + _dynamic_func_call(fmt, __dynamic_ibdev_dbg, \ >>>> + dev, fmt, ##__VA_ARGS__) >>>> + >>>> #define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ >>>> groupsize, buf, len, ascii) \ >>>> _dynamic_func_call_no_desc(__builtin_constant_p(prefix_str) ? prefix_str : "hexdump", \ >>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h >>>> index 9b9e17bcc201..f796e2d44425 100644 >>>> +++ b/include/rdma/ib_verbs.h >>>> @@ -72,6 +72,40 @@ extern struct workqueue_struct *ib_wq; >>>> extern struct workqueue_struct *ib_comp_wq; >>>> extern struct workqueue_struct *ib_comp_unbound_wq; >>>> >>>> +__printf(3, 4) __cold >>>> +void ibdev_printk(const char *level, const struct ib_device *ibdev, >>>> + const char *format, ...); >>>> +__printf(2, 3) __cold >>>> +void ibdev_emerg(const struct ib_device *ibdev, const char *format, ...); >>>> +__printf(2, 3) __cold >>>> +void ibdev_alert(const struct ib_device *ibdev, const char *format, ...); >>>> +__printf(2, 3) __cold >>>> +void ibdev_crit(const struct ib_device *ibdev, const char *format, ...); >>>> +__printf(2, 3) __cold >>>> +void ibdev_err(const struct ib_device *ibdev, const char *format, ...); >>>> +__printf(2, 3) __cold >>>> +void ibdev_warn(const struct ib_device *ibdev, const char *format, ...); >>>> +__printf(2, 3) __cold >>>> +void ibdev_notice(const struct ib_device *ibdev, const char *format, ...); >>>> +__printf(2, 3) __cold >>>> +void ibdev_info(const struct ib_device *ibdev, const char *format, ...); >>>> + >>>> +#if defined(CONFIG_DYNAMIC_DEBUG) >>>> +#define ibdev_dbg(__dev, format, args...) \ >>>> +do { \ >>>> + dynamic_ibdev_dbg(__dev, format, ##args); \ >>>> +} while (0) > > This is weird too, why the while()? No real reason I guess, I'll remove. > >>>> +#elif defined(DEBUG) >>>> +#define ibdev_dbg(__dev, format, args...) \ >>>> + ibdev_printk(KERN_DEBUG, __dev, format, ##args) >>>> +#else >>>> +#define ibdev_dbg(__dev, format, args...) \ >>>> +({ \ >>>> + if (0) \ >>> >>> Why isn't "empty macro" enough here and we need "if (0)"? >> >> I was wondering as well :). >> This is copied from the netdev_dbg counterpart, my best guess is some kind of >> compilation checks in case debug is disabled but I can't even convince myself.. > > It is a trick to get the compiler to do a format & type check even > when debug is disabled. > > Personally I prefer the much clearer non #define version > > __printf(2, 3) > static inline void ibdev_dbg(const struct ib_device *dev, const char > *format, ...) {} > > Instead of the macro tricks. Wouldn't that lose the format type checking? For example passing a pointer to %d?
On Tue, Apr 02, 2019 at 10:05:00AM +0300, Gal Pressman wrote: > >>>> +#elif defined(DEBUG) > >>>> +#define ibdev_dbg(__dev, format, args...) \ > >>>> + ibdev_printk(KERN_DEBUG, __dev, format, ##args) > >>>> +#else > >>>> +#define ibdev_dbg(__dev, format, args...) \ > >>>> +({ \ > >>>> + if (0) \ > >>> > >>> Why isn't "empty macro" enough here and we need "if (0)"? > >> > >> I was wondering as well :). > >> This is copied from the netdev_dbg counterpart, my best guess is some kind of > >> compilation checks in case debug is disabled but I can't even convince myself.. > > > > It is a trick to get the compiler to do a format & type check even > > when debug is disabled. > > > > Personally I prefer the much clearer non #define version > > > > __printf(2, 3) > > static inline void ibdev_dbg(const struct ib_device *dev, const char > > *format, ...) {} > > > > Instead of the macro tricks. > > Wouldn't that lose the format type checking? For example passing a pointer to %d? No, the static inline has the __printf attribute so the compiler checks it even though the inline is empty Jason
On 02-Apr-19 14:57, Jason Gunthorpe wrote: > On Tue, Apr 02, 2019 at 10:05:00AM +0300, Gal Pressman wrote: >>>>>> +#elif defined(DEBUG) >>>>>> +#define ibdev_dbg(__dev, format, args...) \ >>>>>> + ibdev_printk(KERN_DEBUG, __dev, format, ##args) >>>>>> +#else >>>>>> +#define ibdev_dbg(__dev, format, args...) \ >>>>>> +({ \ >>>>>> + if (0) \ >>>>> >>>>> Why isn't "empty macro" enough here and we need "if (0)"? >>>> >>>> I was wondering as well :). >>>> This is copied from the netdev_dbg counterpart, my best guess is some kind of >>>> compilation checks in case debug is disabled but I can't even convince myself.. >>> >>> It is a trick to get the compiler to do a format & type check even >>> when debug is disabled. >>> >>> Personally I prefer the much clearer non #define version >>> >>> __printf(2, 3) >>> static inline void ibdev_dbg(const struct ib_device *dev, const char >>> *format, ...) {} >>> >>> Instead of the macro tricks. >> >> Wouldn't that lose the format type checking? For example passing a pointer to %d? > > No, the static inline has the __printf attribute so the compiler checks it even > though the inline is empty Thanks, will change.
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 7421ec4883fb..a300218d3f99 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -147,6 +147,66 @@ static int ib_security_change(struct notifier_block *nb, unsigned long event, static void ib_policy_change_task(struct work_struct *work); static DECLARE_WORK(ib_policy_change_work, ib_policy_change_task); +static void __ibdev_printk(const char *level, const struct ib_device *ibdev, + struct va_format *vaf) +{ + if (ibdev && ibdev->dev.parent) + dev_printk_emit(level[1] - '0', + ibdev->dev.parent, + "%s %s %s: %pV", + dev_driver_string(ibdev->dev.parent), + dev_name(ibdev->dev.parent), + dev_name(&ibdev->dev), + vaf); + else if (ibdev) + printk("%s%s: %pV", + level, dev_name(&ibdev->dev), vaf); + else + printk("%s(NULL ib_device): %pV", level, vaf); +} + +void ibdev_printk(const char *level, const struct ib_device *ibdev, + const char *format, ...) +{ + struct va_format vaf; + va_list args; + + va_start(args, format); + + vaf.fmt = format; + vaf.va = &args; + + __ibdev_printk(level, ibdev, &vaf); + + va_end(args); +} +EXPORT_SYMBOL(ibdev_printk); + +#define define_ibdev_printk_level(func, level) \ +void func(const struct ib_device *ibdev, const char *fmt, ...) \ +{ \ + struct va_format vaf; \ + va_list args; \ + \ + va_start(args, fmt); \ + \ + vaf.fmt = fmt; \ + vaf.va = &args; \ + \ + __ibdev_printk(level, ibdev, &vaf); \ + \ + va_end(args); \ +} \ +EXPORT_SYMBOL(func); + +define_ibdev_printk_level(ibdev_emerg, KERN_EMERG); +define_ibdev_printk_level(ibdev_alert, KERN_ALERT); +define_ibdev_printk_level(ibdev_crit, KERN_CRIT); +define_ibdev_printk_level(ibdev_err, KERN_ERR); +define_ibdev_printk_level(ibdev_warn, KERN_WARNING); +define_ibdev_printk_level(ibdev_notice, KERN_NOTICE); +define_ibdev_printk_level(ibdev_info, KERN_INFO); + static struct notifier_block ibdev_lsm_nb = { .notifier_call = ib_security_change, }; diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index c2be029b9b53..6c809440f319 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -71,6 +71,13 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, const struct net_device *dev, const char *fmt, ...); +struct ib_device; + +extern __printf(3, 4) +void __dynamic_ibdev_dbg(struct _ddebug *descriptor, + const struct ib_device *ibdev, + const char *fmt, ...); + #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ static struct _ddebug __aligned(8) \ __attribute__((section("__verbose"))) name = { \ @@ -154,6 +161,10 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor, _dynamic_func_call(fmt, __dynamic_netdev_dbg, \ dev, fmt, ##__VA_ARGS__) +#define dynamic_ibdev_dbg(dev, fmt, ...) \ + _dynamic_func_call(fmt, __dynamic_ibdev_dbg, \ + dev, fmt, ##__VA_ARGS__) + #define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ groupsize, buf, len, ascii) \ _dynamic_func_call_no_desc(__builtin_constant_p(prefix_str) ? prefix_str : "hexdump", \ diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 9b9e17bcc201..f796e2d44425 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -72,6 +72,40 @@ extern struct workqueue_struct *ib_wq; extern struct workqueue_struct *ib_comp_wq; extern struct workqueue_struct *ib_comp_unbound_wq; +__printf(3, 4) __cold +void ibdev_printk(const char *level, const struct ib_device *ibdev, + const char *format, ...); +__printf(2, 3) __cold +void ibdev_emerg(const struct ib_device *ibdev, const char *format, ...); +__printf(2, 3) __cold +void ibdev_alert(const struct ib_device *ibdev, const char *format, ...); +__printf(2, 3) __cold +void ibdev_crit(const struct ib_device *ibdev, const char *format, ...); +__printf(2, 3) __cold +void ibdev_err(const struct ib_device *ibdev, const char *format, ...); +__printf(2, 3) __cold +void ibdev_warn(const struct ib_device *ibdev, const char *format, ...); +__printf(2, 3) __cold +void ibdev_notice(const struct ib_device *ibdev, const char *format, ...); +__printf(2, 3) __cold +void ibdev_info(const struct ib_device *ibdev, const char *format, ...); + +#if defined(CONFIG_DYNAMIC_DEBUG) +#define ibdev_dbg(__dev, format, args...) \ +do { \ + dynamic_ibdev_dbg(__dev, format, ##args); \ +} while (0) +#elif defined(DEBUG) +#define ibdev_dbg(__dev, format, args...) \ + ibdev_printk(KERN_DEBUG, __dev, format, ##args) +#else +#define ibdev_dbg(__dev, format, args...) \ +({ \ + if (0) \ + ibdev_printk(KERN_DEBUG, __dev, format, ##args); \ +}) +#endif + union ib_gid { u8 raw[16]; struct { diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 7bdf98c37e91..dfcf6cfa1c70 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -37,6 +37,8 @@ #include <linux/device.h> #include <linux/netdevice.h> +#include <rdma/ib_verbs.h> + extern struct _ddebug __start___verbose[]; extern struct _ddebug __stop___verbose[]; @@ -636,6 +638,44 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg); #endif +#if IS_ENABLED(CONFIG_INFINIBAND) + +void __dynamic_ibdev_dbg(struct _ddebug *descriptor, + const struct ib_device *ibdev, const char *fmt, ...) +{ + struct va_format vaf; + va_list args; + + BUG_ON(!descriptor); + BUG_ON(!fmt); + + va_start(args, fmt); + + vaf.fmt = fmt; + vaf.va = &args; + + if (ibdev && ibdev->dev.parent) { + char buf[PREFIX_SIZE]; + + dev_printk_emit(LOGLEVEL_DEBUG, ibdev->dev.parent, + "%s%s %s %s: %pV", + dynamic_emit_prefix(descriptor, buf), + dev_driver_string(ibdev->dev.parent), + dev_name(ibdev->dev.parent), + dev_name(&ibdev->dev), + &vaf); + } else if (ibdev) { + printk(KERN_DEBUG "%s: %pV", dev_name(&ibdev->dev), &vaf); + } else { + printk(KERN_DEBUG "(NULL ib_device): %pV", &vaf); + } + + va_end(args); +} +EXPORT_SYMBOL(__dynamic_ibdev_dbg); + +#endif + #define DDEBUG_STRING_SIZE 1024 static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE];
Similarly to dev/netdev/etc printk helpers, add standard printk helpers for the RDMA subsystem. Example output: efa 0000:00:06.0 efa_0: Hello World! efa_0: Hello World! (no parent device set) (NULL ib_device): Hello World! (ibdev is NULL) Cc: Jason Baron <jbaron@akamai.com> Signed-off-by: Gal Pressman <galpress@amazon.com> --- drivers/infiniband/core/device.c | 60 ++++++++++++++++++++++++++++++++++++++++ include/linux/dynamic_debug.h | 11 ++++++++ include/rdma/ib_verbs.h | 34 +++++++++++++++++++++++ lib/dynamic_debug.c | 40 +++++++++++++++++++++++++++ 4 files changed, 145 insertions(+)