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: <20210428010031.571623-1-jim.cromie@gmail.com> <20210428010031.571623-2-jim.cromie@gmail.com>
 <f2e4773e-45a2-c4cb-24cb-fc8119f10a3a@akamai.com>
In-Reply-To: <f2e4773e-45a2-c4cb-24cb-fc8119f10a3a@akamai.com>
From:   jim.cromie@gmail.com
Date:   Sat, 1 May 2021 20:49:35 -0600
Message-ID: <CAJfuBxwubk2DatnVrHU1C6WiYNKW1CcuQZe29vToMov4Ch0dHA@mail.gmail.com>
Subject: Re: [PATCH 1/2] dyndbg: avoid calling dyndbg_emit_prefix when it has
 no work
To:     Jason Baron <jbaron@akamai.com>
Cc:     Greg KH <gregkh@linuxfoundation.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Apr 29, 2021 at 3:03 PM Jason Baron <jbaron@akamai.com> wrote:
>
> Hi Jim,
>
> On 4/27/21 9:00 PM, Jim Cromie wrote:
> > Wrap function in a static-inline one, which checks flags to avoid
> > calling the function unnecessarily.
> >
> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > ---
> >  include/linux/dynamic_debug.h | 9 +++++++++
> >  lib/dynamic_debug.c           | 9 ++++++++-
> >  2 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> > index a57ee75342cf..173535e725f7 100644
> > --- a/include/linux/dynamic_debug.h
> > +++ b/include/linux/dynamic_debug.h
> > @@ -32,6 +32,15 @@ struct _ddebug {
> >  #define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
> >  #define _DPRINTK_FLAGS_INCL_LINENO   (1<<3)
> >  #define _DPRINTK_FLAGS_INCL_TID              (1<<4)
> > +
> > +#define _DPRINTK_FLAGS_INCL_ANYSITE          \
> > +     (_DPRINTK_FLAGS_INCL_MODNAME            \
> > +      | _DPRINTK_FLAGS_INCL_FUNCNAME         \
> > +      | _DPRINTK_FLAGS_INCL_LINENO)
> > +#define _DPRINTK_FLAGS_INCL_ANY                      \
> > +     (_DPRINTK_FLAGS_INCL_ANYSITE            \
> > +      | _DPRINTK_FLAGS_INCL_TID)
> > +
>
> I'm not sure it's worth having an unused define here by dynamic_debug.c.
>
> I would prefer to just have _DPRINTK_FLAGS_INCL_ANY that has all the flags
> in a single define.


I have another patch to emit_prefix that uses !ANYSITE to return early
if only TID is wanted
but I can split that out,
or maybe I can pull the other patch forward out of the dd-diet-plan
set Im working.


>
> hmmm - looking at __dynamic_emit_prefix() it starts by doing:
>
>
>  589 static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
>  590 {
>  591         int pos_after_tid;
>  592         int pos = 0;
>  593
>  594         *buf = '\0';
>
>
> So now we are missing the string termination if no flags are set...

yes thats wrong.  looks like I can hoist that init into the caller


> Thanks,
>
> -Jason
>

thanks
Jim