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>
In-Reply-To: <20200608112150.7ohrax6pzpi7ss2l@holly.lan>
From:   jim.cromie@gmail.com
Date:   Tue, 9 Jun 2020 13:59:41 -0600
Message-ID: <CAJfuBxwHOyvOcSB8005=z1G_-Hux1UxaLOutOX1SE2wYONSsrg@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 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

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)

>
> Daniel.
>
> >
> > Also require verbose>=2 for several other debug printks that are a bit
> > too chatty for typical needs;
> >
> > ddebug_change() prints changes, once per modified callsite.  Since
> > queries like "+p" will enable ~2300 callsites in a typical laptop, a
> > user probably doesnt need to see them often.  ddebug_exec_queries()
> > still summarizes with verbose=1.
> >
> > ddebug_(add|remove)_module() also print 1 line per action on a module,
> > not needed by typical modprobe user.
> >
> > This leaves verbose=1 better focussed on the >control parsing process.
> >
> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > ---