Re: [PATCH v3 1/7] Documentation: dynamic-debug: Add description of level bitmask

Greg Kroah-Hartman
2020-06-10T12:26:57Z

On Wed, Jun 10, 2020 at 01:29:20PM +0300, Stanimir Varbanov wrote:
> Hi Greg,
>
> On 6/9/20 2:16 PM, Greg Kroah-Hartman wrote:
> > On Tue, Jun 09, 2020 at 01:45:58PM +0300, Stanimir Varbanov wrote:
> >> This adds description of the level bitmask feature.
> >>
> >> Cc: Jonathan Corbet <corbet@lwn.net> (maintainer:DOCUMENTATION)
> >>
> >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> >> ---
> >> Documentation/admin-guide/dynamic-debug-howto.rst | 10 ++++++++++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> >> index 0dc2eb8e44e5..c2b751fc8a17 100644
> >> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> >> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> >> @@ -208,6 +208,12 @@ line
> >> line -1605 // the 1605 lines from line 1 to line 1605
> >> line 1600- // all lines from line 1600 to the end of the file
> >>
> >> +level
> >> + The given level will be a bitmask ANDed with the level of the each ``pr_debug()``
> >> + callsite. This will allow to group debug messages and show only those of the
> >> + same level. The -p flag takes precedence over the given level. Note that we can
> >> + have up to five groups of debug messages.
> >
> > As was pointed out, this isn't a "level", it's some arbitrary type of
> > "grouping".
>
> Yes, it is grouping of KERN_DEBUG level messages by importance (my
> fault, I put incorrect name). What is important is driver author
> decision. Usually when the driver is huge and has a lot of debug
> messages it is not practical to enable all of them to chasing a
> particular bug or issue. You know that debugging (printk) add delays
> which could hide or rise additional issue(s) which would complicate
> debug and waste time.

That is why it is possible to turn on and off debugging messages on a
function/line basis already. Why not just use that instead?

> For the Venus driver I have defined three groups of KERN_DEBUG - low,
> medium and high (again the driver author(s) will decide what the
> importance is depending on his past experience).
>
> There is another point where the debugging is made by person who is not
> familiar with the driver code. In that case he/she cannot enable lines
> or range of lines because he don't know the details. Here the grouping
> by importance could help.

And they will really know what "low/medium/high" are?

Anyway, that makes a bit more sense, but the documentation could use a
lot more in order to describe this type of behavior, and what is
expected by both driver authors, and users of the interface.

thanks,

greg k-h