diff mbox series

[v5,2/4] xen: move debugtrace coding to common/debugtrace.c

Message ID 20190905113955.24870-3-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series xen: debugtrace cleanup and per-cpu buffer support | expand

Commit Message

Jürgen Groß Sept. 5, 2019, 11:39 a.m. UTC
Instead of living in drivers/char/console.c move the debugtrace
related coding to a new file common/debugtrace.c

No functional change, code movement only.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/common/Makefile        |   1 +
 xen/common/debugtrace.c    | 181 +++++++++++++++++++++++++++++++++++++++++++++
 xen/drivers/char/console.c | 179 +-------------------------------------------
 3 files changed, 183 insertions(+), 178 deletions(-)
 create mode 100644 xen/common/debugtrace.c

Comments

Jan Beulich Sept. 5, 2019, 12:20 p.m. UTC | #1
On 05.09.2019 13:39, Juergen Gross wrote:
> --- /dev/null
> +++ b/xen/common/debugtrace.c
> @@ -0,0 +1,181 @@
> +/******************************************************************************
> + * debugtrace.c
> + *
> + * Debugtrace for Xen
> + */
> +
> +
> +#include <xen/console.h>
> +#include <xen/init.h>
> +#include <xen/keyhandler.h>
> +#include <xen/lib.h>
> +#include <xen/mm.h>
> +#include <xen/serial.h>
> +#include <xen/spinlock.h>
> +#include <xen/watchdog.h>
> +
> +#define DEBUG_TRACE_ENTRY_SIZE   1024
> +
> +/* Send output direct to console, or buffer it? */
> +static volatile int debugtrace_send_to_console;
> +
> +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];
> +static DEFINE_SPINLOCK(debugtrace_lock);
> +integer_param("debugtrace", debugtrace_kilobytes);
> +
> +static void debugtrace_dump_worker(void)

And another remark here too, despite my prior ack: By moving this into
its own file, the debugtrace_ prefixes of static symbols now all
become redundant, at least as far as their occurrence in e.g. call
stacks goes (where they'd be prefixes by the disambiguating source
file name). But I know we've got ample other examples where this is
also the case, and I also know there are contrary opinions on the
matter, so this is not strictly a request for further change.

Jan
Jürgen Groß Sept. 5, 2019, 12:32 p.m. UTC | #2
On 05.09.19 14:20, Jan Beulich wrote:
> On 05.09.2019 13:39, Juergen Gross wrote:
>> --- /dev/null
>> +++ b/xen/common/debugtrace.c
>> @@ -0,0 +1,181 @@
>> +/******************************************************************************
>> + * debugtrace.c
>> + *
>> + * Debugtrace for Xen
>> + */
>> +
>> +
>> +#include <xen/console.h>
>> +#include <xen/init.h>
>> +#include <xen/keyhandler.h>
>> +#include <xen/lib.h>
>> +#include <xen/mm.h>
>> +#include <xen/serial.h>
>> +#include <xen/spinlock.h>
>> +#include <xen/watchdog.h>
>> +
>> +#define DEBUG_TRACE_ENTRY_SIZE   1024
>> +
>> +/* Send output direct to console, or buffer it? */
>> +static volatile int debugtrace_send_to_console;
>> +
>> +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];
>> +static DEFINE_SPINLOCK(debugtrace_lock);
>> +integer_param("debugtrace", debugtrace_kilobytes);
>> +
>> +static void debugtrace_dump_worker(void)
> 
> And another remark here too, despite my prior ack: By moving this into
> its own file, the debugtrace_ prefixes of static symbols now all
> become redundant, at least as far as their occurrence in e.g. call
> stacks goes (where they'd be prefixes by the disambiguating source
> file name). But I know we've got ample other examples where this is
> also the case, and I also know there are contrary opinions on the
> matter, so this is not strictly a request for further change.

I'm one of the "contrary opinion" guys. :-)

So I'd rather keep the prefix.


Juergen
diff mbox series

Patch

diff --git a/xen/common/Makefile b/xen/common/Makefile
index eddda5daa6..62b34e69e9 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -4,6 +4,7 @@  obj-y += bsearch.o
 obj-$(CONFIG_CORE_PARKING) += core_parking.o
 obj-y += cpu.o
 obj-y += cpupool.o
+obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
 obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
 obj-y += domctl.o
 obj-y += domain.o
diff --git a/xen/common/debugtrace.c b/xen/common/debugtrace.c
new file mode 100644
index 0000000000..c1ee3f45b9
--- /dev/null
+++ b/xen/common/debugtrace.c
@@ -0,0 +1,181 @@ 
+/******************************************************************************
+ * debugtrace.c
+ *
+ * Debugtrace for Xen
+ */
+
+
+#include <xen/console.h>
+#include <xen/init.h>
+#include <xen/keyhandler.h>
+#include <xen/lib.h>
+#include <xen/mm.h>
+#include <xen/serial.h>
+#include <xen/spinlock.h>
+#include <xen/watchdog.h>
+
+#define DEBUG_TRACE_ENTRY_SIZE   1024
+
+/* Send output direct to console, or buffer it? */
+static volatile int debugtrace_send_to_console;
+
+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];
+static DEFINE_SPINLOCK(debugtrace_lock);
+integer_param("debugtrace", debugtrace_kilobytes);
+
+static void debugtrace_dump_worker(void)
+{
+    if ( (debugtrace_bytes == 0) || !debugtrace_used )
+        return;
+
+    printk("debugtrace_dump() starting\n");
+
+    /* Print oldest portion of the ring. */
+    if ( debugtrace_buf[debugtrace_prd] != '\0' )
+        console_serial_puts(&debugtrace_buf[debugtrace_prd],
+                            debugtrace_bytes - debugtrace_prd);
+
+    /* Print youngest portion of the ring. */
+    debugtrace_buf[debugtrace_prd] = '\0';
+    console_serial_puts(&debugtrace_buf[0], debugtrace_prd);
+
+    memset(debugtrace_buf, '\0', debugtrace_bytes);
+    debugtrace_prd = 0;
+    debugtrace_last_entry_buf[0] = 0;
+
+    printk("debugtrace_dump() finished\n");
+}
+
+static void debugtrace_toggle(void)
+{
+    unsigned long flags;
+
+    watchdog_disable();
+    spin_lock_irqsave(&debugtrace_lock, flags);
+
+    /*
+     * Dump the buffer *before* toggling, in case the act of dumping the
+     * buffer itself causes more printk() invocations.
+     */
+    printk("debugtrace_printk now writing to %s.\n",
+           !debugtrace_send_to_console ? "console": "buffer");
+    if ( !debugtrace_send_to_console )
+        debugtrace_dump_worker();
+
+    debugtrace_send_to_console = !debugtrace_send_to_console;
+
+    spin_unlock_irqrestore(&debugtrace_lock, flags);
+    watchdog_enable();
+
+}
+
+void debugtrace_dump(void)
+{
+    unsigned long flags;
+
+    watchdog_disable();
+    spin_lock_irqsave(&debugtrace_lock, flags);
+
+    debugtrace_dump_worker();
+
+    spin_unlock_irqrestore(&debugtrace_lock, flags);
+    watchdog_enable();
+}
+
+static void debugtrace_add_to_buf(char *buf)
+{
+    char *p;
+
+    for ( p = buf; *p != '\0'; p++ )
+    {
+        debugtrace_buf[debugtrace_prd++] = *p;
+        if ( debugtrace_prd == debugtrace_bytes )
+            debugtrace_prd = 0;
+    }
+}
+
+void debugtrace_printk(const char *fmt, ...)
+{
+    static char buf[DEBUG_TRACE_ENTRY_SIZE];
+    static unsigned int count, last_count, last_prd;
+
+    char          cntbuf[24];
+    va_list       args;
+    unsigned long flags;
+    unsigned int nr;
+
+    if ( debugtrace_bytes == 0 )
+        return;
+
+    debugtrace_used = 1;
+
+    spin_lock_irqsave(&debugtrace_lock, flags);
+
+    va_start(args, fmt);
+    nr = vsnprintf(buf, sizeof(buf), fmt, args);
+    va_end(args);
+
+    if ( debugtrace_send_to_console )
+    {
+        unsigned int n = scnprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
+
+        console_serial_puts(cntbuf, n);
+        console_serial_puts(buf, nr);
+    }
+    else
+    {
+        if ( strcmp(buf, debugtrace_last_entry_buf) )
+        {
+            last_prd = debugtrace_prd;
+            last_count = ++count;
+            safe_strcpy(debugtrace_last_entry_buf, buf);
+            snprintf(cntbuf, sizeof(cntbuf), "%u ", count);
+        }
+        else
+        {
+            debugtrace_prd = last_prd;
+            snprintf(cntbuf, sizeof(cntbuf), "%u-%u ", last_count, ++count);
+        }
+        debugtrace_add_to_buf(cntbuf);
+        debugtrace_add_to_buf(buf);
+    }
+
+    spin_unlock_irqrestore(&debugtrace_lock, flags);
+}
+
+static void debugtrace_key(unsigned char key)
+{
+    debugtrace_toggle();
+}
+
+static int __init debugtrace_init(void)
+{
+    int order;
+    unsigned int kbytes, bytes;
+
+    /* Round size down to next power of two. */
+    while ( (kbytes = (debugtrace_kilobytes & (debugtrace_kilobytes-1))) != 0 )
+        debugtrace_kilobytes = kbytes;
+
+    bytes = debugtrace_kilobytes << 10;
+    if ( bytes == 0 )
+        return 0;
+
+    order = get_order_from_bytes(bytes);
+    debugtrace_buf = alloc_xenheap_pages(order, 0);
+    ASSERT(debugtrace_buf != NULL);
+
+    memset(debugtrace_buf, '\0', bytes);
+
+    debugtrace_bytes = bytes;
+
+    register_keyhandler('T', debugtrace_key,
+                        "toggle debugtrace to console/buffer", 0);
+
+    return 0;
+}
+__initcall(debugtrace_init);
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 8df627c84a..7f29190eaf 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1160,184 +1160,7 @@  int printk_ratelimit(void)
 
 /*
  * **************************************************************
- * *************** Serial console ring buffer *******************
- * **************************************************************
- */
-
-#ifdef CONFIG_DEBUG_TRACE
-
-#define DEBUG_TRACE_ENTRY_SIZE   1024
-
-/* Send output direct to console, or buffer it? */
-static volatile int debugtrace_send_to_console;
-
-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];
-static DEFINE_SPINLOCK(debugtrace_lock);
-integer_param("debugtrace", debugtrace_kilobytes);
-
-static void debugtrace_dump_worker(void)
-{
-    if ( (debugtrace_bytes == 0) || !debugtrace_used )
-        return;
-
-    printk("debugtrace_dump() starting\n");
-
-    /* Print oldest portion of the ring. */
-    if ( debugtrace_buf[debugtrace_prd] != '\0' )
-        console_serial_puts(&debugtrace_buf[debugtrace_prd],
-                            debugtrace_bytes - debugtrace_prd);
-
-    /* Print youngest portion of the ring. */
-    debugtrace_buf[debugtrace_prd] = '\0';
-    console_serial_puts(&debugtrace_buf[0], debugtrace_prd);
-
-    memset(debugtrace_buf, '\0', debugtrace_bytes);
-    debugtrace_prd = 0;
-    debugtrace_last_entry_buf[0] = 0;
-
-    printk("debugtrace_dump() finished\n");
-}
-
-static void debugtrace_toggle(void)
-{
-    unsigned long flags;
-
-    watchdog_disable();
-    spin_lock_irqsave(&debugtrace_lock, flags);
-
-    /*
-     * Dump the buffer *before* toggling, in case the act of dumping the
-     * buffer itself causes more printk() invocations.
-     */
-    printk("debugtrace_printk now writing to %s.\n",
-           !debugtrace_send_to_console ? "console": "buffer");
-    if ( !debugtrace_send_to_console )
-        debugtrace_dump_worker();
-
-    debugtrace_send_to_console = !debugtrace_send_to_console;
-
-    spin_unlock_irqrestore(&debugtrace_lock, flags);
-    watchdog_enable();
-
-}
-
-void debugtrace_dump(void)
-{
-    unsigned long flags;
-
-    watchdog_disable();
-    spin_lock_irqsave(&debugtrace_lock, flags);
-
-    debugtrace_dump_worker();
-
-    spin_unlock_irqrestore(&debugtrace_lock, flags);
-    watchdog_enable();
-}
-
-static void debugtrace_add_to_buf(char *buf)
-{
-    char *p;
-
-    for ( p = buf; *p != '\0'; p++ )
-    {
-        debugtrace_buf[debugtrace_prd++] = *p;
-        if ( debugtrace_prd == debugtrace_bytes )
-            debugtrace_prd = 0;
-    }
-}
-
-void debugtrace_printk(const char *fmt, ...)
-{
-    static char buf[DEBUG_TRACE_ENTRY_SIZE];
-    static unsigned int count, last_count, last_prd;
-
-    char          cntbuf[24];
-    va_list       args;
-    unsigned long flags;
-    unsigned int nr;
-
-    if ( debugtrace_bytes == 0 )
-        return;
-
-    debugtrace_used = 1;
-
-    spin_lock_irqsave(&debugtrace_lock, flags);
-
-    va_start(args, fmt);
-    nr = vscnprintf(buf, sizeof(buf), fmt, args);
-    va_end(args);
-
-    if ( debugtrace_send_to_console )
-    {
-        unsigned int n = scnprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
-
-        console_serial_puts(cntbuf, n);
-        console_serial_puts(buf, nr);
-    }
-    else
-    {
-        if ( strcmp(buf, debugtrace_last_entry_buf) )
-        {
-            last_prd = debugtrace_prd;
-            last_count = ++count;
-            safe_strcpy(debugtrace_last_entry_buf, buf);
-            snprintf(cntbuf, sizeof(cntbuf), "%u ", count);
-        }
-        else
-        {
-            debugtrace_prd = last_prd;
-            snprintf(cntbuf, sizeof(cntbuf), "%u-%u ", last_count, ++count);
-        }
-        debugtrace_add_to_buf(cntbuf);
-        debugtrace_add_to_buf(buf);
-    }
-
-    spin_unlock_irqrestore(&debugtrace_lock, flags);
-}
-
-static void debugtrace_key(unsigned char key)
-{
-    debugtrace_toggle();
-}
-
-static int __init debugtrace_init(void)
-{
-    int order;
-    unsigned int kbytes, bytes;
-
-    /* Round size down to next power of two. */
-    while ( (kbytes = (debugtrace_kilobytes & (debugtrace_kilobytes-1))) != 0 )
-        debugtrace_kilobytes = kbytes;
-
-    bytes = debugtrace_kilobytes << 10;
-    if ( bytes == 0 )
-        return 0;
-
-    order = get_order_from_bytes(bytes);
-    debugtrace_buf = alloc_xenheap_pages(order, 0);
-    ASSERT(debugtrace_buf != NULL);
-
-    memset(debugtrace_buf, '\0', bytes);
-
-    debugtrace_bytes = bytes;
-
-    register_keyhandler('T', debugtrace_key,
-                        "toggle debugtrace to console/buffer", 0);
-
-    return 0;
-}
-__initcall(debugtrace_init);
-
-#endif /* !CONFIG_DEBUG_TRACE */
-
-
-/*
- * **************************************************************
- * *************** Debugging/tracing/error-report ***************
+ * ********************** Error-report **************************
  * **************************************************************
  */