Re: [PATCH rdma-next v4 01/12] RDMA/core: Introduce RDMA subsystem ibdev_* print functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> >> Signed-off-by: Gal Pressman <galpress@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux