On 19-Mar-19 21:33, Jason Gunthorpe wrote:
> On Mon, Mar 18, 2019 at 11:47:32PM +0200, Gal Pressman wrote:
>> On 14-Mar-19 17:31, Jason Gunthorpe wrote:
>>> On Thu, Mar 14, 2019 at 05:09:22PM +0200, Gal Pressman wrote:
>>>>>> +#define efa_dbg(_dev, format, ...) \
>>>>>> + dev_dbg(_dev, "(pid %d) %s: " format, current->pid, \
>>>>>> + __func__, ##__VA_ARGS__)
>>>>>> +#define efa_info(_dev, format, ...) \
>>>>>> + dev_info(_dev, "(pid %d) %s: " format, current->pid, \
>>>>>> + __func__, ##__VA_ARGS__)
>>>>>> +#define efa_warn(_dev, format, ...) \
>>>>>> + dev_warn(_dev, "(pid %d) %s: " format, current->pid, \
>>>>>> + __func__, ##__VA_ARGS__)
>>>>>> +#define efa_err(_dev, format, ...) \
>>>>>> + dev_err(_dev, "(pid %d) %s: " format, current->pid, \
>>>>>> + __func__, ##__VA_ARGS__)
>>>>>> +#define efa_err_rl(_dev, format, ...) \
>>>>>> + dev_err_ratelimited(_dev, "(pid %d) %s: " format, current->pid, \
>>>>>> + __func__, ##__VA_ARGS__)
>>>>>
>>>>> Every time when I see such debug prints, it makes me wonder if they
>>>>> actually needed. Anyway "current->pid" will print wrong output for any
>>>>> kernel threads. I know that you are not supporting kverbs, but still
>>>>> don't think that it is right thing to print.
>>>>
>>>> What's the reason pid is wrong for kernel threads?
>>>> I found it quite useful to see the process id while debugging, at least for
>>>> userspace applications. Is there anything other we can use instead of
>>>> current->pid that would work for both?
>>>
>>> Again, I'd really like it if the three new drivers could get together
>>> and have core code that does this stuff sensibly and
>>> consistently. netdev has stuff like this already
>>>
>>> If pid logging makes sense here then it does for all..
>>
>> I'm fine with that, is this an acceptable format for the subsystem? Should I
>> remove the pid?
>
> I haven't seen it yet, but I guess we have some core kernel code doing
> the pid now?
Right, I'll start working on ibdev_{err,warn,...} helpers.
