xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Juergen Gross <jgross@suse.com>
To: xen-devel@lists.xenproject.org
Cc: Juergen Gross <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>
Subject: [Xen-devel] [PATCH v6 4/5] xen: add per-cpu buffer option to debugtrace
Date: Mon,  9 Sep 2019 09:35:56 +0200	[thread overview]
Message-ID: <20190909073557.16168-5-jgross@suse.com> (raw)
In-Reply-To: <20190909073557.16168-1-jgross@suse.com>

debugtrace is normally writing trace entries into a single trace
buffer. There are cases where this is not optimal, e.g. when hunting
a bug which requires writing lots of trace entries and one cpu is
stuck. This will result in other cpus filling the trace buffer and
finally overwriting the interesting trace entries of the hanging cpu.

In order to be able to debug such situations add the capability to use
per-cpu trace buffers. This can be selected by specifying the
debugtrace boot parameter with the modifier "cpu:", like:

  debugtrace=cpu:16

At the same time switch the parsing function to accept size modifiers
(e.g. 4M or 1G).

Printing out the trace entries is done for each buffer in order to
minimize the effort needed during printing. As each entry is prefixed
with its sequence number sorting the entries can easily be done when
analyzing them.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- only allocate buffer if not already done so
V4:
- unsigned int -> unsigned long (Jan Beulich)
- replace check for bytes < PAGE_SIZE by !bytes (Jan Beulich)
- print info which buffer allocation failed (Jan Beulich)
- replace switch by if in cpu notifier handler (Jan Beulich)
V5:
- don't silently ignore trailing characters when parsing buffer size
  (Jan Beulich)
- limit scope of some variables (Jan Beulich)
- adjust error message format (Jan Beulich)
V6:
- move calculation of final debugtrace_bytes value to parameter parsing
---
 docs/misc/xen-command-line.pandoc |   7 +-
 xen/common/debugtrace.c           | 158 +++++++++++++++++++++++++++++---------
 2 files changed, 127 insertions(+), 38 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 7c72e31032..832797e2e2 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -644,12 +644,13 @@ over the PCI busses sequentially) or by PCI device (must be on segment 0).
 Limits the number lines printed in Xen stack traces.
 
 ### debugtrace
-> `= <integer>`
+> `= [cpu:]<size>`
 
 > Default: `128`
 
-Specify the size of the console debug trace buffer in KiB. The debug
-trace feature is only enabled in debugging builds of Xen.
+Specify the size of the console debug trace buffer. By specifying `cpu:`
+additionally a trace buffer of the specified size is allocated per cpu.
+The debug trace feature is only enabled in debugging builds of Xen.
 
 ### dma_bits
 > `= <integer>`
diff --git a/xen/common/debugtrace.c b/xen/common/debugtrace.c
index 51ab41f7a7..669b2102ab 100644
--- a/xen/common/debugtrace.c
+++ b/xen/common/debugtrace.c
@@ -6,6 +6,7 @@
 
 
 #include <xen/console.h>
+#include <xen/cpu.h>
 #include <xen/init.h>
 #include <xen/keyhandler.h>
 #include <xen/lib.h>
@@ -23,35 +24,79 @@ struct debugtrace_data {
 };
 
 static struct debugtrace_data *dt_data;
+static DEFINE_PER_CPU(struct debugtrace_data *, dt_cpu_data);
 
-static unsigned int debugtrace_kilobytes = 128;
-static unsigned long debugtrace_bytes;
+static unsigned long debugtrace_bytes = 128 << 10;
+static bool debugtrace_per_cpu;
 static bool debugtrace_buf_empty;
 static bool debugtrace_used;
 static DEFINE_SPINLOCK(debugtrace_lock);
-integer_param("debugtrace", debugtrace_kilobytes);
 
-static void debugtrace_dump_worker(void)
+static int __init debugtrace_parse_param(const char *s)
 {
-    if ( !debugtrace_used )
+    unsigned long bytes;
+
+    if ( !strncmp(s, "cpu:", 4) )
+    {
+        debugtrace_per_cpu = true;
+        s += 4;
+    }
+    bytes = parse_size_and_unit(s, &s);
+
+    if ( *s )
+        return -EINVAL;
+
+    debugtrace_bytes = MAX(bytes, PAGE_SIZE);
+
+    /* Round size down to next power of two. */
+    while ( (bytes = (debugtrace_bytes & (debugtrace_bytes - 1))) != 0 )
+        debugtrace_bytes = bytes;
+
+    return 0;
+}
+custom_param("debugtrace", debugtrace_parse_param);
+
+static void debugtrace_dump_buffer(struct debugtrace_data *data,
+                                   const char *which)
+{
+    if ( !data )
         return;
 
-    printk("debugtrace_dump() starting\n");
+    printk("debugtrace_dump() %s buffer starting\n", which);
 
     /* Print oldest portion of the ring. */
-    if ( dt_data->buf[dt_data->prd] != '\0' )
-        console_serial_puts(&dt_data->buf[dt_data->prd],
-                            debugtrace_bytes - dt_data->prd);
+    if ( data->buf[data->prd] != '\0' )
+        console_serial_puts(&data->buf[data->prd],
+                            debugtrace_bytes - data->prd);
 
     /* Print youngest portion of the ring. */
-    dt_data->buf[dt_data->prd] = '\0';
-    console_serial_puts(&dt_data->buf[0], dt_data->prd);
+    data->buf[data->prd] = '\0';
+    console_serial_puts(&data->buf[0], data->prd);
 
-    memset(dt_data->buf, '\0', debugtrace_bytes);
-    dt_data->prd = 0;
-    debugtrace_buf_empty = true;
+    memset(data->buf, '\0', debugtrace_bytes);
+    data->prd = 0;
 
-    printk("debugtrace_dump() finished\n");
+    printk("debugtrace_dump() %s buffer finished\n", which);
+}
+
+static void debugtrace_dump_worker(void)
+{
+    unsigned int cpu;
+
+    if ( !debugtrace_used )
+        return;
+
+    debugtrace_dump_buffer(dt_data, "global");
+
+    for ( cpu = 0; cpu < nr_cpu_ids; cpu++ )
+    {
+        char buf[16];
+
+        snprintf(buf, sizeof(buf), "cpu %u", cpu);
+        debugtrace_dump_buffer(per_cpu(dt_cpu_data, cpu), buf);
+    }
+
+    debugtrace_buf_empty = true;
 }
 
 static void debugtrace_toggle(void)
@@ -91,28 +136,33 @@ void debugtrace_dump(void)
 
 static void debugtrace_add_to_buf(char *buf)
 {
+    struct debugtrace_data *data;
     char *p;
 
+    data = debugtrace_per_cpu ? this_cpu(dt_cpu_data) : dt_data;
+
     for ( p = buf; *p != '\0'; p++ )
     {
-        dt_data->buf[dt_data->prd++] = *p;
-        if ( dt_data->prd == debugtrace_bytes )
-            dt_data->prd = 0;
+        data->buf[data->prd++] = *p;
+        if ( data->prd == debugtrace_bytes )
+            data->prd = 0;
     }
 }
 
 void debugtrace_printk(const char *fmt, ...)
 {
     static char buf[1024], last_buf[1024];
-    static unsigned int count, last_count;
+    static unsigned int count, last_count, last_cpu;
     static unsigned long last_prd;
 
     char          cntbuf[24];
     va_list       args;
     unsigned long flags;
     unsigned int nr;
+    struct debugtrace_data *data;
 
-    if ( !dt_data )
+    data = debugtrace_per_cpu ? this_cpu(dt_cpu_data) : dt_data;
+    if ( !data )
         return;
 
     debugtrace_used = true;
@@ -132,17 +182,20 @@ void debugtrace_printk(const char *fmt, ...)
     }
     else
     {
+        unsigned int cpu = debugtrace_per_cpu ? smp_processor_id() : 0;
+
         if ( debugtrace_buf_empty || strcmp(buf, last_buf) )
         {
             debugtrace_buf_empty = false;
-            last_prd = dt_data->prd;
+            last_prd = data->prd;
             last_count = ++count;
+            last_cpu = cpu;
             safe_strcpy(last_buf, buf);
             snprintf(cntbuf, sizeof(cntbuf), "%u ", count);
         }
         else
         {
-            dt_data->prd = last_prd;
+            data->prd = last_prd;
             snprintf(cntbuf, sizeof(cntbuf), "%u-%u ", last_count, ++count);
         }
         debugtrace_add_to_buf(cntbuf);
@@ -157,34 +210,69 @@ static void debugtrace_key(unsigned char key)
     debugtrace_toggle();
 }
 
-static int __init debugtrace_init(void)
+static void debugtrace_alloc_buffer(struct debugtrace_data **ptr,
+                                    unsigned int cpu)
 {
     int order;
-    unsigned long kbytes;
     struct debugtrace_data *data;
 
-    /* Round size down to next power of two. */
-    while ( (kbytes = (debugtrace_kilobytes & (debugtrace_kilobytes-1))) != 0 )
-        debugtrace_kilobytes = kbytes;
-
-    debugtrace_bytes = debugtrace_kilobytes << 10;
-    if ( debugtrace_bytes == 0 )
-        return 0;
+    if ( *ptr )
+        return;
 
     order = get_order_from_bytes(debugtrace_bytes);
     data = alloc_xenheap_pages(order, 0);
     if ( !data )
-        return -ENOMEM;
+    {
+        if ( debugtrace_per_cpu )
+            printk("CPU%u: failed to allocate debugtrace buffer\n", cpu);
+        else
+            printk("failed to allocate debugtrace buffer\n");
+        return;
+    }
 
-    debugtrace_bytes = PAGE_SIZE << order;
-    memset(data, '\0', debugtrace_bytes);
-    debugtrace_bytes -= sizeof(*data);
+    memset(data, '\0', debugtrace_bytes + sizeof(*data));
 
     dt_data = data;
+    *ptr = data;
+}
+
+static int debugtrace_cpu_callback(struct notifier_block *nfb,
+                                   unsigned long action, void *hcpu)
+{
+    unsigned int cpu = (unsigned long)hcpu;
+
+    /* Buffers are only ever allocated, never freed. */
+    if ( action == CPU_UP_PREPARE )
+        debugtrace_alloc_buffer(&per_cpu(dt_cpu_data, cpu), cpu);
+
+    return 0;
+}
+
+static struct notifier_block debugtrace_nfb = {
+    .notifier_call = debugtrace_cpu_callback
+};
+
+static int __init debugtrace_init(void)
+{
+    unsigned int cpu;
+
+    if ( !debugtrace_bytes )
+        return 0;
 
     register_keyhandler('T', debugtrace_key,
                         "toggle debugtrace to console/buffer", 0);
 
+    debugtrace_bytes -= sizeof(struct debugtrace_data);
+
+    if ( debugtrace_per_cpu )
+    {
+        for_each_online_cpu ( cpu )
+            debugtrace_alloc_buffer(&per_cpu(dt_cpu_data, cpu), cpu);
+        register_cpu_notifier(&debugtrace_nfb);
+    }
+    else
+        debugtrace_alloc_buffer(&dt_data, 0);
+
     return 0;
 }
 __initcall(debugtrace_init);
-- 
2.16.4


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

  parent reply	other threads:[~2019-09-09  7:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-09  7:35 [Xen-devel] [PATCH v6 0/5] xen: debugtrace cleanup and per-cpu buffer support Juergen Gross
2019-09-09  7:35 ` [Xen-devel] [PATCH v6 1/5] xen: fix debugtrace clearing Juergen Gross
2019-09-09  7:53   ` Jan Beulich
2019-09-09  8:11     ` Jan Beulich
2019-09-09  8:19     ` Juergen Gross
2019-09-09  7:35 ` [Xen-devel] [PATCH v6 2/5] xen: move debugtrace coding to common/debugtrace.c Juergen Gross
2019-09-09  7:35 ` [Xen-devel] [PATCH v6 3/5] xen: refactor debugtrace data Juergen Gross
2019-09-09  7:35 ` Juergen Gross [this message]
2019-09-09  7:59   ` [Xen-devel] [PATCH v6 4/5] xen: add per-cpu buffer option to debugtrace Jan Beulich
2019-09-09  8:04     ` Jan Beulich
2019-09-09  8:15       ` Juergen Gross
2019-09-09  7:35 ` [Xen-devel] [PATCH v6 5/5] xen: add debugtrace entry when entry count is wrapping Juergen Gross
2019-09-09  8:19   ` Jan Beulich
2019-09-09  8:22     ` Juergen Gross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190909073557.16168-5-jgross@suse.com \
    --to=jgross@suse.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).