diff mbox series

[for-next,1/2] RDMA/core: Introduce ratelimited ibdev printk functions

Message ID 20190730151834.70993-2-galpress@amazon.com (mailing list archive)
State Superseded
Headers show
Series Ratelimited ibdev printk functions | expand

Commit Message

Gal Pressman July 30, 2019, 3:18 p.m. UTC
Add ratelimited helpers to the ibdev_* printk functions.
Implementation inspired by counterpart dev_*_ratelimited functions.

Signed-off-by: Gal Pressman <galpress@amazon.com>
---
 include/rdma/ib_verbs.h | 51 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Leon Romanovsky July 30, 2019, 3:41 p.m. UTC | #1
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@amazon.com>
> ---
>  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.

Thanks


> +#define ibdev_dbg_ratelimited(ibdev, fmt, ...)                          \
> +do {                                                                    \
> +	static DEFINE_RATELIMIT_STATE(_rs,                              \
> +				      DEFAULT_RATELIMIT_INTERVAL,       \
> +				      DEFAULT_RATELIMIT_BURST);         \
> +	if (__ratelimit(&_rs))                                          \
> +		ibdev_printk(KERN_DEBUG, ibdev, fmt, ##__VA_ARGS__);    \
> +} while (0)
> +#else
> +__printf(2, 3) __cold
> +static inline
> +void ibdev_dbg_ratelimited(const struct ib_device *ibdev, const char *format, ...) {}
> +#endif
> +
>  union ib_gid {
>  	u8	raw[16];
>  	struct {
> --
> 2.22.0
>
Gal Pressman July 31, 2019, 7:22 a.m. UTC | #2
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@amazon.com>
>> ---
>>  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
Leon Romanovsky July 31, 2019, 7:41 a.m. UTC | #3
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@amazon.com>
> >> ---
> >>  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
Gal Pressman July 31, 2019, 10:51 a.m. UTC | #4
On 31/07/2019 10:41, Leon Romanovsky wrote:
> 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@amazon.com>
>>>> ---
>>>>  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.

DEBUG exists since the first commit in the tree, and is used in various parts of
the kernel (mlx5 as well). Do you think it should be removed from the kernel?

Regarding combination of both, I don't think DEBUG is related to
CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.
Leon Romanovsky July 31, 2019, 11:46 a.m. UTC | #5
On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote:
> On 31/07/2019 10:41, Leon Romanovsky wrote:
> > 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@amazon.com>
> >>>> ---
> >>>>  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.
>
> DEBUG exists since the first commit in the tree, and is used in various parts of
> the kernel (mlx5 as well). Do you think it should be removed from the kernel?

It is gradually removed when it is spotted, I'll send a patch for mlx5 now.

>
> Regarding combination of both, I don't think DEBUG is related to
> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.

I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm
asking YOU to provide us real and in-tree scenario where DEBUG will
exists and CONFIG_DYNAMIC_DEBUG won't.

Thanks
Gal Pressman July 31, 2019, 12:56 p.m. UTC | #6
On 31/07/2019 14:46, Leon Romanovsky wrote:
> On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote:
>> On 31/07/2019 10:41, Leon Romanovsky wrote:
>>> 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@amazon.com>
>>>>>> ---
>>>>>>  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.
>>
>> DEBUG exists since the first commit in the tree, and is used in various parts of
>> the kernel (mlx5 as well). Do you think it should be removed from the kernel?
> 
> It is gradually removed when it is spotted, I'll send a patch for mlx5 now.

Was there an on-list discussion regarding removal of DEBUG usage? Can you please
share a link?
If so, I agree the DEBUG part should be removed.

> 
>>
>> Regarding combination of both, I don't think DEBUG is related to
>> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
>> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.
> 
> I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm
> asking YOU to provide us real and in-tree scenario where DEBUG will
> exists and CONFIG_DYNAMIC_DEBUG won't.

What's any of this has to do with in-tree? This code and defines are part of the
tree.

The use case doesn't matter, it's a valid permutation. Is there anything that
stops a user from building the kernel this way?
Leon Romanovsky July 31, 2019, 1:33 p.m. UTC | #7
On Wed, Jul 31, 2019 at 03:56:55PM +0300, Gal Pressman wrote:
> On 31/07/2019 14:46, Leon Romanovsky wrote:
> > On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote:
> >> On 31/07/2019 10:41, Leon Romanovsky wrote:
> >>> 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@amazon.com>
> >>>>>> ---
> >>>>>>  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.
> >>
> >> DEBUG exists since the first commit in the tree, and is used in various parts of
> >> the kernel (mlx5 as well). Do you think it should be removed from the kernel?
> >
> > It is gradually removed when it is spotted, I'll send a patch for mlx5 now.
>
> Was there an on-list discussion regarding removal of DEBUG usage? Can you please
> share a link?

Sorry, but no, I didn't know that I need to save all discussions I see
in the mailing lists.

> If so, I agree the DEBUG part should be removed.
>
> >
> >>
> >> Regarding combination of both, I don't think DEBUG is related to
> >> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
> >> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.
> >
> > I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm
> > asking YOU to provide us real and in-tree scenario where DEBUG will
> > exists and CONFIG_DYNAMIC_DEBUG won't.
>
> What's any of this has to do with in-tree? This code and defines are part of the
> tree.
>
> The use case doesn't matter, it's a valid permutation. Is there anything that
> stops a user from building the kernel this way?

Like everything else, nothing stops from you to do any modifications to
the source code, before you will build. We are talking about in-tree
builds and distro kernels.

Thanks
Gal Pressman July 31, 2019, 2:19 p.m. UTC | #8
On 31/07/2019 16:33, Leon Romanovsky wrote:
> On Wed, Jul 31, 2019 at 03:56:55PM +0300, Gal Pressman wrote:
>> On 31/07/2019 14:46, Leon Romanovsky wrote:
>>> On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote:
>>>> On 31/07/2019 10:41, Leon Romanovsky wrote:
>>>>> 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@amazon.com>
>>>>>>>> ---
>>>>>>>>  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.
>>>>
>>>> DEBUG exists since the first commit in the tree, and is used in various parts of
>>>> the kernel (mlx5 as well). Do you think it should be removed from the kernel?
>>>
>>> It is gradually removed when it is spotted, I'll send a patch for mlx5 now.
>>
>> Was there an on-list discussion regarding removal of DEBUG usage? Can you please
>> share a link?
> 
> Sorry, but no, I didn't know that I need to save all discussions I see
> in the mailing lists.

Trying to understand whether "It is gradually removed when it is spotted" is a
well known guideline by the community, should checkpatch produce a warning for this?

> 
>> If so, I agree the DEBUG part should be removed.
>>
>>>
>>>>
>>>> Regarding combination of both, I don't think DEBUG is related to
>>>> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
>>>> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.
>>>
>>> I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm
>>> asking YOU to provide us real and in-tree scenario where DEBUG will
>>> exists and CONFIG_DYNAMIC_DEBUG won't.
>>
>> What's any of this has to do with in-tree? This code and defines are part of the
>> tree.
>>
>> The use case doesn't matter, it's a valid permutation. Is there anything that
>> stops a user from building the kernel this way?
> 
> Like everything else, nothing stops from you to do any modifications to
> the source code, before you will build. We are talking about in-tree
> builds and distro kernels.

Last I checked turning on DEBUG doesn't require source code changes?
Leon Romanovsky July 31, 2019, 2:50 p.m. UTC | #9
On Wed, Jul 31, 2019 at 05:19:41PM +0300, Gal Pressman wrote:
> On 31/07/2019 16:33, Leon Romanovsky wrote:
> > On Wed, Jul 31, 2019 at 03:56:55PM +0300, Gal Pressman wrote:
> >> On 31/07/2019 14:46, Leon Romanovsky wrote:
> >>> On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote:
> >>>> On 31/07/2019 10:41, Leon Romanovsky wrote:
> >>>>> 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@amazon.com>
> >>>>>>>> ---
> >>>>>>>>  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.
> >>>>
> >>>> DEBUG exists since the first commit in the tree, and is used in various parts of
> >>>> the kernel (mlx5 as well). Do you think it should be removed from the kernel?
> >>>
> >>> It is gradually removed when it is spotted, I'll send a patch for mlx5 now.
> >>
> >> Was there an on-list discussion regarding removal of DEBUG usage? Can you please
> >> share a link?
> >
> > Sorry, but no, I didn't know that I need to save all discussions I see
> > in the mailing lists.
>
> Trying to understand whether "It is gradually removed when it is spotted" is a
> well known guideline by the community, should checkpatch produce a warning for this?

I didn't see checks in checkpatch about tabs<->spaces mix either which you
pointed for hns guys.

>
> >
> >> If so, I agree the DEBUG part should be removed.
> >>
> >>>
> >>>>
> >>>> Regarding combination of both, I don't think DEBUG is related to
> >>>> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
> >>>> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.
> >>>
> >>> I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm
> >>> asking YOU to provide us real and in-tree scenario where DEBUG will
> >>> exists and CONFIG_DYNAMIC_DEBUG won't.
> >>
> >> What's any of this has to do with in-tree? This code and defines are part of the
> >> tree.
> >>
> >> The use case doesn't matter, it's a valid permutation. Is there anything that
> >> stops a user from building the kernel this way?
> >
> > Like everything else, nothing stops from you to do any modifications to
> > the source code, before you will build. We are talking about in-tree
> > builds and distro kernels.
>
> Last I checked turning on DEBUG doesn't require source code changes?

Exciting, how did you enable DEBUG without recompiling source code?
Maybe you find a way to enable DEBUG on running kernel?

And how did it come that v5.3 kernel was compiled with DEBUG but
without DYNAMIC_DEBUG?

Thanks
Gal Pressman July 31, 2019, 3:26 p.m. UTC | #10
On 31/07/2019 17:50, Leon Romanovsky wrote:
> On Wed, Jul 31, 2019 at 05:19:41PM +0300, Gal Pressman wrote:
>> On 31/07/2019 16:33, Leon Romanovsky wrote:
>>> On Wed, Jul 31, 2019 at 03:56:55PM +0300, Gal Pressman wrote:
>>>> On 31/07/2019 14:46, Leon Romanovsky wrote:
>>>>> On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote:
>>>>>> On 31/07/2019 10:41, Leon Romanovsky wrote:
>>>>>>> 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@amazon.com>
>>>>>>>>>> ---
>>>>>>>>>>  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.
>>>>>>
>>>>>> DEBUG exists since the first commit in the tree, and is used in various parts of
>>>>>> the kernel (mlx5 as well). Do you think it should be removed from the kernel?
>>>>>
>>>>> It is gradually removed when it is spotted, I'll send a patch for mlx5 now.
>>>>
>>>> Was there an on-list discussion regarding removal of DEBUG usage? Can you please
>>>> share a link?
>>>
>>> Sorry, but no, I didn't know that I need to save all discussions I see
>>> in the mailing lists.
>>
>> Trying to understand whether "It is gradually removed when it is spotted" is a
>> well known guideline by the community, should checkpatch produce a warning for this?
> 
> I didn't see checks in checkpatch about tabs<->spaces mix either which you
> pointed for hns guys.

Ofcourse there are, this patch was full of checkpatch warnings.
But that's not the point, you're avoiding answering a simple question: is DEBUG
usage discouraged by the community?

> 
>>
>>>
>>>> If so, I agree the DEBUG part should be removed.
>>>>
>>>>>
>>>>>>
>>>>>> Regarding combination of both, I don't think DEBUG is related to
>>>>>> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
>>>>>> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.
>>>>>
>>>>> I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm
>>>>> asking YOU to provide us real and in-tree scenario where DEBUG will
>>>>> exists and CONFIG_DYNAMIC_DEBUG won't.
>>>>
>>>> What's any of this has to do with in-tree? This code and defines are part of the
>>>> tree.
>>>>
>>>> The use case doesn't matter, it's a valid permutation. Is there anything that
>>>> stops a user from building the kernel this way?
>>>
>>> Like everything else, nothing stops from you to do any modifications to
>>> the source code, before you will build. We are talking about in-tree
>>> builds and distro kernels.
>>
>> Last I checked turning on DEBUG doesn't require source code changes?
> 
> Exciting, how did you enable DEBUG without recompiling source code?

Recompiling source code != changing source code.
You can turn on DEBUG when compiling the kernel (i.e running make) with no
source code changes (again, last I checked, did this change lately?).

> Maybe you find a way to enable DEBUG on running kernel?
> 
> And how did it come that v5.3 kernel was compiled with DEBUG but
> without DYNAMIC_DEBUG?

Change CONFIG_DYNAMIC_DEBUG=n in your .config and pass DEBUG to make.
Leon Romanovsky July 31, 2019, 5:54 p.m. UTC | #11
On Wed, Jul 31, 2019 at 06:26:16PM +0300, Gal Pressman wrote:
> On 31/07/2019 17:50, Leon Romanovsky wrote:
> > On Wed, Jul 31, 2019 at 05:19:41PM +0300, Gal Pressman wrote:
> >> On 31/07/2019 16:33, Leon Romanovsky wrote:
> >>> On Wed, Jul 31, 2019 at 03:56:55PM +0300, Gal Pressman wrote:
> >>>> On 31/07/2019 14:46, Leon Romanovsky wrote:
> >>>>> On Wed, Jul 31, 2019 at 01:51:05PM +0300, Gal Pressman wrote:
> >>>>>> On 31/07/2019 10:41, Leon Romanovsky wrote:
> >>>>>>> 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@amazon.com>
> >>>>>>>>>> ---
> >>>>>>>>>>  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.
> >>>>>>
> >>>>>> DEBUG exists since the first commit in the tree, and is used in various parts of
> >>>>>> the kernel (mlx5 as well). Do you think it should be removed from the kernel?
> >>>>>
> >>>>> It is gradually removed when it is spotted, I'll send a patch for mlx5 now.
> >>>>
> >>>> Was there an on-list discussion regarding removal of DEBUG usage? Can you please
> >>>> share a link?
> >>>
> >>> Sorry, but no, I didn't know that I need to save all discussions I see
> >>> in the mailing lists.
> >>
> >> Trying to understand whether "It is gradually removed when it is spotted" is a
> >> well known guideline by the community, should checkpatch produce a warning for this?
> >
> > I didn't see checks in checkpatch about tabs<->spaces mix either which you
> > pointed for hns guys.
>
> Ofcourse there are, this patch was full of checkpatch warnings.
> But that's not the point, you're avoiding answering a simple question: is DEBUG
> usage discouraged by the community?

Yes, I said it a couple of times, "community" is not excited to see
debug code in final code. The expectation that submitted code works
and doesn't need extra debug. If it is not true, such code is not ready
to be merged.

Anyway, simple grep in our subsystem shows that this "DEBUG" in use in one
place only (fmr) and it predates git era.
_  kernel git:(rdma-next) git grep DEBUG drivers/infiniband/ include/rdma/ | grep ifdef

There is no new code with this "DEBUG" flag.

>
> >
> >>
> >>>
> >>>> If so, I agree the DEBUG part should be removed.
> >>>>
> >>>>>
> >>>>>>
> >>>>>> Regarding combination of both, I don't think DEBUG is related to
> >>>>>> CONFIG_DYNAMIC_DEBUG. DEBUG is a generic debug flag (not necessarily to prints)
> >>>>>> while CONFIG_DYNAMIC_DEBUG is specific to the dynamic debug prints infrastructure.
> >>>>>
> >>>>> I know exactly what DEBUG and CONFIG_DYNAMIC_DEBUG mean, but I'm
> >>>>> asking YOU to provide us real and in-tree scenario where DEBUG will
> >>>>> exists and CONFIG_DYNAMIC_DEBUG won't.
> >>>>
> >>>> What's any of this has to do with in-tree? This code and defines are part of the
> >>>> tree.
> >>>>
> >>>> The use case doesn't matter, it's a valid permutation. Is there anything that
> >>>> stops a user from building the kernel this way?
> >>>
> >>> Like everything else, nothing stops from you to do any modifications to
> >>> the source code, before you will build. We are talking about in-tree
> >>> builds and distro kernels.
> >>
> >> Last I checked turning on DEBUG doesn't require source code changes?
> >
> > Exciting, how did you enable DEBUG without recompiling source code?
>
> Recompiling source code != changing source code.

I'm as a kernel developer don't see the difference.

> You can turn on DEBUG when compiling the kernel (i.e running make) with no
> source code changes (again, last I checked, did this change lately?).
>
> > Maybe you find a way to enable DEBUG on running kernel?
> >
> > And how did it come that v5.3 kernel was compiled with DEBUG but
> > without DYNAMIC_DEBUG?
>
> Change CONFIG_DYNAMIC_DEBUG=n in your .config and pass DEBUG to make.

I asked for a real example and not for allnoconfig compilation.
diff mbox series

Patch

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)
+#define ibdev_dbg_ratelimited(ibdev, fmt, ...)                          \
+do {                                                                    \
+	static DEFINE_RATELIMIT_STATE(_rs,                              \
+				      DEFAULT_RATELIMIT_INTERVAL,       \
+				      DEFAULT_RATELIMIT_BURST);         \
+	if (__ratelimit(&_rs))                                          \
+		ibdev_printk(KERN_DEBUG, ibdev, fmt, ##__VA_ARGS__);    \
+} while (0)
+#else
+__printf(2, 3) __cold
+static inline
+void ibdev_dbg_ratelimited(const struct ib_device *ibdev, const char *format, ...) {}
+#endif
+
 union ib_gid {
 	u8	raw[16];
 	struct {