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: <20200617162536.611386-1-jim.cromie@gmail.com> <20200617162536.611386-22-jim.cromie@gmail.com>
 <20200618124400.GA7536@alley>
In-Reply-To: <20200618124400.GA7536@alley>
From:   jim.cromie@gmail.com
Date:   Thu, 18 Jun 2020 08:54:58 -0600
Message-ID: <CAJfuBxyw7v=uQFMLHbsP_MAub7DFZOto6SnU71upXZDcK9L9QQ@mail.gmail.com>
Subject: Re: [PATCH v3 19/21] dyndbg: extend ddebug_parse_flags to accept
 optional leading filter-flags
To:     Petr Mladek <pmladek@suse.com>
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>,
        Jonathan Corbet <corbet@lwn.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Will Deacon <will@kernel.org>,
        Orson Zhai <orson.zhai@unisoc.com>,
        Linux Documentation List <linux-doc@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jun 18, 2020 at 6:44 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Wed 2020-06-17 10:25:34, Jim Cromie wrote:
> > Change ddebug_parse_flags to accept optional filterflags before the
> > required operator [-+=].  Read the flags into the filter_flags
> > parameter added in the previous patch.  So this now supplies the
> > filterflags to ddebug_exec_query.
> >
> > filterflags work like query terms, they constrain what callsites get
> > matched before theyre modified.  So like a query, they can be empty.
> >
> > Filterflags let you read callsite's flagstate, including results of
> > previous modifications, and require that certain flags are set, before
> > modifying the callsite further.
> >
> > So you can build up sets of callsites by marking them with a
> > particular flagstate, for example 'fmlt', then enable that set in a
> > batch.
> >
> >   echo fmlt+p >control
> >
> > Naturally you can use almost any combo of flags you want for marking,
> > and can mark several different sets with different patterns.  And then
> > you can activate them in a bunch:
> >
> >   echo 'ft+p; mt+p; lt+p;' >control
> >
> > + * Parse `str' as a flags-spec, ie: [pfmlt_]*[-+=][pfmlt_]+
>
> This interface is simply _horrible_ and I do not see a point in this feature!!!
>
> I as a normal dynamic debug user am interested into:
>
>    + enabling/disabling messages from a given module/file/line/function
>    + list of available modules/files/lines/functions
>    + list of enabled modules/files/lines/functions
>
> I do not understand why I would ever want to do something like:
>
>    + enable messages that print module name and line number
>    + disable message that does not print a module name

messages dont print them, the flags do, according to USER CHOICE.
a developer who is deeply familiar with the code doesnt need to
see most of it in the logs, average user might need them to comprehend things.

>
> In fact, IMHO, all the 'flmt' flags were a wrong idea and nobody
> really needed them. This information in not needed by other
> printk() messages. Why pr_debug() would need them?
> They just made the code and interface complicated.
>

it looks like they landed fully formed in lib/dynamic_debug.c
probably because that was a unification of several different print
debug systems.

you are free to set them globally:
echo +fmlt >control

or just the ones youre using
echo up+fmlt >control

> Please, stop all this non-sense!!!
>
> Best Regards,
> Petr