Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200613155738.2249399-1-jim.cromie@gmail.com>
 <20200613155738.2249399-21-jim.cromie@gmail.com> <20200616134507.GO31238@alley>
In-Reply-To: <20200616134507.GO31238@alley>
From:   jim.cromie@gmail.com
Date:   Tue, 16 Jun 2020 15:13:10 -0600
Message-ID: <CAJfuBxxvejGJtyQfcPmG7i6p6NAESjTuBPHeGDUwTVkg9_JfzQ@mail.gmail.com>
Subject: Re: [PATCH v2 20/24] dyndbg: WIP towards debug-print-class based
 callsite controls
To:     Petr Mladek <pmladek@suse.com>,
        Stanimir Varbanov <stanimir.varbanov@linaro.org>
Cc:     Jason Baron <jbaron@akamai.com>,
        LKML <linux-kernel@vger.kernel.org>, akpm@linuxfoundation.org,
        Greg KH <gregkh@linuxfoundation.org>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

hi Petr,

On Tue, Jun 16, 2020 at 7:45 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Sat 2020-06-13 09:57:34, Jim Cromie wrote:
> > There are *lots* of ad-hoc debug printing solutions in kernel,
> > this is a 1st attempt at providing a common mechanism for many of them.
>
> I agree that it might make sense to provide some common mechanism.
>
>
> > Basically, there are 2 styles of debug printing:
> > - levels, with increasing verbosity, 1-10 forex
> > - bits/flags, independently controlling separate groups of dprints
> >
> > This patch does bits/flags only.
> >
> > proposed API:
> >
> > Usage model is for a module developer to create N exclusive subsets of
> > pr_debug()s by changing some of them to pr_debug_n(1,) .. pr_debug_n(N,).
> > Each callsite must be a single print-class, with 0 default.
> >
> > No multi-type classification ala pr_debug_M(1|2, ...) is contemplated.
> >
> >   Qfoo() { echo module foo $* >/proc/dynamic_debug/control }
> >   Qfoo +p             # all groups, including default 0
> >   Qfoo mflags 1 +p    # only group 1
> >   Qfoo mflags 12 +p   # TBD[1]: groups 1 or 2
> >   Qfoo mflags 0 +p    # ignored atm TBD[2]
> >   Qfoo mflags af +p   # TBD[3]: groups a or f (10 or 15)
>
> My problem with this approach is that it is too generic. Each class
> would have different meaning in each subsystem.
>

I think generic is a feature.

subsystem and module are the organizational level
where print-classes would be sensibly defined.
Thats why I required a module query-term with mflags,
 so that no mflags query could operate on all the callsites.
I might go further to prohibit a wildcard in the module query-term value,
so its absolutely locked in to a specific module.

maybe there would be consensus about having 1 or 2 kernel-wide print-classes,
but other than something akin to stdin, stdout, stderr, I cant think
of what it would look like.


> It might help to replace any existing variants. But it would be hard
> for developers debugging the code. They would need to study/remember
> the meaning of these groups for particular subsystems. They would
> need to set different values for different messages.
>
> Could you please provide more details about the potential users?

Ive ccd Stanimir, who wanted HI MID LOW classifications on some of his
debug prints.
Im doing the simplest possible thing that might work for him.

> Would be possible to find some common patterns and common
> meaning of the groups?

probably should start with anti-patterns.

KISAP - simple as possible
I offer exclusive classes only, no this or that.
prclass = 0 is reserved for every current use.

If module authors think they need many  print-classes, rethink.
control output exposes whole structure of code,
file and function names are chosen to convey organization,
a small set of queries will recreate most arbitrary print-classes

use the echo >control interface only.
I briefly thought about checking module debug parameters,
that now sounds like madness

Any chunk of code with N pr-debug* callsites, however badly architected,
can be completely controlled by N separate queries.
for better code, its much smaller.


ISTM the only sane way to allow external modules to control their own
dynamic debug callsites is to expose ddebug_exec_queries,
and let them issue the callsite modifications they want.

then, they can map any updates to their debug-flags storage
onto a pair of on-off queries for each bit.

Lastly, Id note that exclusive classes doesnt mean levels (debug++, debug--)
cant be handled.
we could reserve pr-classes 1-9 to mean all inclusive below N.

Or that meaning could be handled by merely issuing the fill-in activations.
In the module that wants debug levels

  echo module foo mflags 4 >control
auto generates same query 3 more times, with mflags 3 flags 2 mflags:1


>
> Best Regards,
> Petr