On Wed, Jul 31, 2019 at 10:22:42AM +0300, Gal Pressman wrote:
> On 30/07/2019 18:41, Leon Romanovsky wrote:
> > On Tue, Jul 30, 2019 at 06:18:33PM +0300, Gal Pressman wrote:
> >> Add ratelimited helpers to the ibdev_* printk functions.
> >> Implementation inspired by counterpart dev_*_ratelimited functions.
> >>
> >> Signed-off-by: Gal Pressman <galpress@xxxxxxxxxx>
> >> ---
> >> include/rdma/ib_verbs.h | 51 +++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 51 insertions(+)
> >>
> >> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> >> index c5f8a9f17063..356e6a105366 100644
> >> --- a/include/rdma/ib_verbs.h
> >> +++ b/include/rdma/ib_verbs.h
> >> @@ -107,6 +107,57 @@ static inline
> >> void ibdev_dbg(const struct ib_device *ibdev, const char *format, ...) {}
> >> #endif
> >>
> >> +#define ibdev_level_ratelimited(ibdev_level, ibdev, fmt, ...) \
> >> +do { \
> >> + static DEFINE_RATELIMIT_STATE(_rs, \
> >> + DEFAULT_RATELIMIT_INTERVAL, \
> >> + DEFAULT_RATELIMIT_BURST); \
> >> + if (__ratelimit(&_rs)) \
> >> + ibdev_level(ibdev, fmt, ##__VA_ARGS__); \
> >> +} while (0)
> >> +
> >> +#define ibdev_emerg_ratelimited(ibdev, fmt, ...) \
> >> + ibdev_level_ratelimited(ibdev_emerg, ibdev, fmt, ##__VA_ARGS__)
> >> +#define ibdev_alert_ratelimited(ibdev, fmt, ...) \
> >> + ibdev_level_ratelimited(ibdev_alert, ibdev, fmt, ##__VA_ARGS__)
> >> +#define ibdev_crit_ratelimited(ibdev, fmt, ...) \
> >> + ibdev_level_ratelimited(ibdev_crit, ibdev, fmt, ##__VA_ARGS__)
> >> +#define ibdev_err_ratelimited(ibdev, fmt, ...) \
> >> + ibdev_level_ratelimited(ibdev_err, ibdev, fmt, ##__VA_ARGS__)
> >> +#define ibdev_warn_ratelimited(ibdev, fmt, ...) \
> >> + ibdev_level_ratelimited(ibdev_warn, ibdev, fmt, ##__VA_ARGS__)
> >> +#define ibdev_notice_ratelimited(ibdev, fmt, ...) \
> >> + ibdev_level_ratelimited(ibdev_notice, ibdev, fmt, ##__VA_ARGS__)
> >> +#define ibdev_info_ratelimited(ibdev, fmt, ...) \
> >> + ibdev_level_ratelimited(ibdev_info, ibdev, fmt, ##__VA_ARGS__)
> >> +
> >> +#if defined(CONFIG_DYNAMIC_DEBUG)
> >> +/* descriptor check is first to prevent flooding with "callbacks suppressed" */
> >> +#define ibdev_dbg_ratelimited(ibdev, fmt, ...) \
> >> +do { \
> >> + static DEFINE_RATELIMIT_STATE(_rs, \
> >> + DEFAULT_RATELIMIT_INTERVAL, \
> >> + DEFAULT_RATELIMIT_BURST); \
> >> + DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
> >> + if (DYNAMIC_DEBUG_BRANCH(descriptor) && __ratelimit(&_rs)) \
> >> + __dynamic_ibdev_dbg(&descriptor, ibdev, fmt, \
> >> + ##__VA_ARGS__); \
> >> +} while (0)
> >> +#elif defined(DEBUG)
> >
> > When will you see this CONFIG_DEBUG set? I suspect only in private
> > out-of-tree builds which we are not really care. Also I can't imagine
> > system with this CONFIG_DEBUG and without CONFIG_DYNAMIC_DEBUG.
>
> This is the common way to handle debug prints, see:
> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/printk.h#L331
> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/device.h#L1493
> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/netdevice.h#L4743
> https://elixir.bootlin.com/linux/v5.2.1/source/include/linux/net.h#L266
I'm more interested to know the real usage of this copy/paste and
understand if it makes sense for drivers/infiniband/* or not.
Not everything in netdev is great and worth to borrow.
Thanks
