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

[Juergen Gross <jgross@suse.com>]

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@suse.com>
> 
> Technically this is fine, so it can have my
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel