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

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



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.


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.


Juergen


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

 


Rackspace

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