Re: [RFC 02/11] Add RoCE driver framework

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3713 bytes --]

On Tue, Sep 20, 2016 at 03:04:12PM +0000, Elior, Ariel wrote:
> > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > On Thu, Sep 15, 2016 at 05:11:03AM +0000, Mintz, Yuval wrote:
> > > > As a summary, I didn't see in your responses any real life example where you will
> > > > need global debug level for your driver.
> > >
> > > Not sure what you you're expecting - a list of BZs /private e-mails where
> > > user reproductions were needed?
> > > You're basically ignoring my claims that such are used, instead wanting
> > > "evidence". I'm not going to try and produce any such.
> >
> > I asked an example and not evidence, where "modprobe your_driver
> > debug=1" will be superior to "modprobe your_driver dyndbg==pmf".
> >
> > https://www.kernel.org/doc/Documentation/dynamic-debug-howto.txt
> Hi,
>
> dyndbg vs module param:
> Dynamic debug has two very nice features: dynamic activation/configuration and per line/file/module/format activation. The module param has neither, but it does have a few merits which I am not sure dyndbg has:
> (1)
> It can activate printouts according to *flow*. A lot of thought has been put into associating the right printout in our driver at the right verbosity level with the right "flow tag" (e.g. QEDR_MSG_INIT, QEDR_MSG_QP). The module parameter accepts a bitmask which allows setting any subset of these flows. This means that with the correct values for the parameter I can open only "init" printouts, or only "Memory Region" printouts, even if these cross multiple files / functions and don't share a common format. Presumably, one would claim that we could add the "flow tag" to the format to every printout according to its flow, but that would encumber the printouts, and also doesn't scale well to printouts which belong to multiple flows, where the current approach allows this (QEDR_MSG_SQ | QEDR_
 MSG_RQ).

Dynamic prints are enabled per-print. The best possible granularity
which you can achieve.

> (2)
> As Yuval pointed out, there are users out there which have no trouble loading a driver with a module parameter, at probe or at kernel boot, but would be at a loss in mounting debugfs and dumping a matchspec script into a node there. As kernel developers, educating users is part of what we do, but it comes with a cost.

You are free to add this parameter to your out-of-tree driver. As I said
before to Yuval your module "debug" is equal to "dyndbg". Your users can
use it and is already available in kernel.

> (3)
> debugfs can be compiled out of the kernel in some codesize sensitive environments, or may not exist in an emergency shell or kdump kernel, whereas the module parameter would always be there.
>
> Simply allowing our module parameter mechanism to be dynamically activated is very straightforward - we planned on adding a debugfs node for that anyway. But I would keep the module parameter for the sake of those less capable users and also for when debugfs is not available as detailed in (3) above.

This is an example why we don't want this module parameter, once added it
will be with us forever.

>
> I certainly see the merits of joining an existing infrastructure instead of implementing our own thing, but I would like to know if there are ways of obtaining the merits I listed for our approach via that infrastructure.
>
> Leon, aside of commenting on the above, can you give an example of a driver which in your opinion does a good job of using dyndbg?

All modern drivers in this subsystem are supporting dyndbg out of the
box.

General note, can you please configure your email to wrap lines? It is
very hard to read, follow and respond to lines with >800 symbols in them.

>
> Thanks,
> Ariel
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]