Re: [PATCH 1/2] dyndbg: avoid calling dyndbg_emit_prefix when it has no work

[jim . cromie]

On Thu, Apr 29, 2021 at 3:03 PM Jason Baron <jbaron@xxxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >  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