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?
