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: <20200605162645.289174-1-jim.cromie@gmail.com> <20200605162645.289174-4-jim.cromie@gmail.com>
 <20200608112150.7ohrax6pzpi7ss2l@holly.lan> <CAJfuBxwHOyvOcSB8005=z1G_-Hux1UxaLOutOX1SE2wYONSsrg@mail.gmail.com>
 <20200610111615.z6vukjsq2aprkug4@holly.lan>
In-Reply-To: <20200610111615.z6vukjsq2aprkug4@holly.lan>
From:   jim.cromie@gmail.com
Date:   Wed, 10 Jun 2020 07:45:47 -0600
Message-ID: <CAJfuBxxqc2qL+Ba=yqD_tWS5Ux3ggS4ouf1eAmeOqGOFrybvNg@mail.gmail.com>
Subject: Re: [PATCH 03/16] dyndbg: refine debug verbosity; 1 is basic, 2 more chatty
To:     Daniel Thompson <daniel.thompson@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

On Wed, Jun 10, 2020 at 5:16 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Tue, Jun 09, 2020 at 01:59:41PM -0600, jim.cromie@gmail.com wrote:
> > On Mon, Jun 8, 2020 at 5:21 AM Daniel Thompson
> > <daniel.thompson@linaro.org> wrote:
> > >
> > > On Fri, Jun 05, 2020 at 10:26:32AM -0600, Jim Cromie wrote:
> > > > The verbose/debug logging done for `cat $MNT/dynamic_debug/control` is
> > > > voluminous (2 per control file entry + 2 per PAGE).  Moreover, it just
> > > > prints pointer and sequence, which is not useful to a dyndbg user.
> > > > So just drop them.
> > >
> > > I'd assumed these messages where to help the dyndbg implementer rather
> > > than the dyndbg user.
> >
> > So I thought I was guilty of adding those noisy pr_info()s in the
> > ddebug_proc_* functions,
> > but I have touched them, changing them to vpr_info().
> > In any case, I dont think theyre useful to the implementer either.
> >
> > If the verbose messages really are useful to help
> > > users who (mis)configure .../control then should the enable/disable
> > > control be shadowed in debugfs to make it easy to find?
> > >
> >
> > I would hesitate to change the API, even if this is just an add-on,
> > without changes to existing.
> > OTOH, I could see it added as /proc/dynamic_debug/verbose
>
> /proc ?
>
> I was assuming that if the verbose output of dynamic debug is useful to
> the person trying to *use* dynamic_debug then it should be in
> /sys/kernel/debug/dynamic_debug/verbose .

You are correct.
/proc/dynamic_debug/control does exist, so that debugfs isnt required,
but not verbose.  I conflated them while typing.

so, if debugfs isnt present, neither is dynamic_debug/verbose,
and run-time verbosity changes arent possible
but dynamic_debug.verbose=1 still works as boot arg
(I believe, I havent built an non-debugfs kernel myself)

>
> If they are only likely useful to the person trying to *implement*
> dynamic_debug itself (or to check that the infrastructure is not broken)
> then there is no reason to add them to debugfs.
>


>
> > with this patch, verbose=1 is better focused on showing the parsing process,
> > to give user more context as to what his query-command is doing
> > verbose=2 additionally shows callsites that match the query, including
> > any unchanged (iirc)
>
> I'm still a little confused by what benefit having two levels of
> verbosity really is. Why does a user need to turn on verbose mode to
> figure out what is happening? Why isn't reading back
> .../dynamic_debug/control (perhaps using grep and friends) sufficient?
>

verbose=2  prints quite a bit more,
during boot,  290 lines vs 16.
After you see it a dozen times, you dont need it anymore.
but I think its still worth keeping
It also is more chatty about callsites changed by ` echo $querycmd > control `

`cat control` shows complete callsite & flagstate,
it cant also show how commands are processed & changes are applied.


>
> Daniel.