[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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, ...)
-        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) )


I'd rather have a bool which will be reset in above condition. This will
avoid problems when count is wrapping.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.