Re: [Xen-devel] [PATCH 1/3] xen: move debugtrace coding to common/debugtrace.c

[Jan Beulich <JBeulich@xxxxxxxx>]

On 29.07.2019 15:30, Juergen Gross wrote:
> On 29.07.19 14:45, Jan Beulich wrote:
>> On 28.07.2019 10:40, Juergen Gross wrote:
>>> -#endif /* !CONFIG_DEBUG_TRACE */
>>> -
>>> -
>>>    /*
>>>     * **************************************************************
>>>     * *************** Debugging/tracing/error-report ***************
>>
>> ... what about this one? There's only panic() between it and the next
>> such comment, and I don't think the "Debugging/tracing" part of it
>> are applicable (anymore).
> 
> True. I'll remove the "Debugging/tracing" part.
> 
>>
>>> --- a/xen/include/xen/console.h
>>> +++ b/xen/include/xen/console.h
>>> @@ -48,4 +48,8 @@ int console_resume(void);
>>>    extern int8_t opt_console_xen;
>>> +/* Issue string via serial line. */
>>> +extern int sercon_handle;
>>> +void sercon_puts(const char *s);
>>
>> I guess avoiding their exposure was one of the reasons the debug trace
>> code lived in the place you move it from. I'm unconvinced non-console
>> code is actually supposed to make use of either, but I'm not opposed
>> enough to nak the change. I don't think though the comment fits well
>> with the variable declaration.
> 
> sercon_handle is used for calling serial_puts(), so maybe instead of
> directly using serial_puts() with sercon_handle I should add a wrapper
> to console.c (e.g. console_serial_puts())? It should be noted that
> serial_puts() is called only in case of debugtrace output toggled to go
> to the console. I guess using serial_puts() in that case is meant to
> avoid too many software layers when doing the output.

Hmm, I'd rather expect this to be used to avoid doing anything else
sercon_puts() does besides calling serial_puts(). These other steps
are also why I think this is to remain a console internal interface.

> It would be
> possible to use sercon_puts() for that case, too, resulting in the
> inability to use debugtrace_printk() in the then additionally needed
> paths (or better: to use it with output redirected to console).
> 
> sercon_puts() could use another wrapper, e.g. console_debug_puts().
> 
> Would you like that better?

Probably not. I wonder whether splitting out this code is really a
good step.

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