Re: [PATCH] kgdb: fix gcc-11 warning on indentation

[Daniel Thompson]

On Mon, Mar 22, 2021 at 10:04:57AM -0700, Doug Anderson wrote:
> > +       if (verbose)                    \
> > +               printk(KERN_INFO a);    \
> > +} while (0)
> > +#define v2printk(a...) do {            \
> > +       if (verbose > 1)                \
> > +               printk(KERN_INFO a);    \
> > +       touch_nmi_watchdog();           \
> 
> This touch_nmi_watchdog() is pretty wonky. I guess maybe the
> assumption is that the "verbose level 2" prints are so chatty that the
> printing might prevent us from touching the NMI watchdog in the way
> that we normally do and thus we need an extra one here?
> 
> ...but, in that case, I think the old code was _wrong_ and that the
> intention was that the touch_nmi_watchdog() should only be if "verose
> > 1" as the indentation implied. There doesn't feel like a reason to
> touch the watchdog if we're not doing anything slow.

I'm not entirely sure I'd like to second guess the intent here. This
macro has been there since this file was introduced but several callers
have been added since then. We have to guess their intent too!

So, whilst I think you are probably right, v2printk() does appears in
places such as the single step test loop which makes it pretty
difficult to decide by inspection whether or not touching the watchdog
is useful.

It's something that could be further examined... but I'd be a little
reluctant to combine it directly with a whitespace change!


Daniel.