Re: [Xen-devel] [PATCH v5 1/4] xen: fix debugtrace clearing

[Juergen Gross <jgross@xxxxxxxx>]

On 05.09.19 14:17, Jan Beulich wrote:
> On 05.09.2019 13:39, Juergen Gross wrote:
> > After dumping the debugtrace buffer it is cleared. This results in some
> > entries not being printed in case the buffer is dumped again before
> > having wrapped.
> >
> > While at it remove the trailing zero byte in the buffer as it is no
> > longer needed. Commit b5e6e1ee8da59f introduced passing the number of
> > chars to be printed in the related interfaces, so the trailing 0 byte
> > is no longer required.
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> Technically this is fine, so it can have my
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> However, ...
>
> > @@ -1173,6 +1175,7 @@ static char        *debugtrace_buf; /* Debug-trace buffer 
> > */
> >   static unsigned int debugtrace_prd; /* Producer index     */
> >   static unsigned int debugtrace_kilobytes = 128, debugtrace_bytes;
> >   static unsigned int debugtrace_used;
> > +static char debugtrace_last_entry_buf[DEBUG_TRACE_ENTRY_SIZE];
> ... this is what I was afraid would happen, but I admit I didn't
> reply in a way previously indicating that I dislike such a
> solution. This is also why, when noticing the issue, I didn't put
> together a patch myself right away. In particular I'm of the
> opinion that the three last_* values would better all stay
> together, and then would better stay inside the only function
> using them.
>
> > @@ -1279,11 +1280,11 @@ void debugtrace_printk(const char *fmt, ...)
> >       }
> >       else
> >       {
> > -        if ( strcmp(buf, last_buf) )
> > +        if ( strcmp(buf, debugtrace_last_entry_buf) )
> Wouldn't moving count to file scope and latching its value into
> a new dump_count when dumping work:
>
>          if ( count == dump_count || strcmp(buf, last_buf) )
>
> work?
I'd rather have a bool which will be reset in above condition. This will
avoid problems when count is wrapping.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel