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? Jason
