On 29.07.2019 16:28, Juergen Gross wrote: > On 29.07.19 16:00, Jan Beulich wrote: >> 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. > > To me it seems a little bit strange to have the buffer dumping using > sercon_puts() while issuing the actual trace entries to console isn't > using it.
I guess I agree. >>> 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. > > I'm not fighting for it, I just thought it would better be put into a > file of its own. > > In case you disagree and others are not pushing for separation I'm fine > to drop this patch. Well, I don't mind the separation as long as it's indeed properly separated in the end. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
