On 03.09.19 12:51, Jan Beulich wrote:
On 28.08.2019 10:00, Juergen Gross wrote:
@@ -24,32 +25,62 @@ struct debugtrace_data_s {
};
static struct debugtrace_data_s *debtr_data;
+static DEFINE_PER_CPU(struct debugtrace_data_s *, debtr_cpu_data);
-static unsigned int debugtrace_kilobytes = 128;
+static unsigned int debugtrace_bytes = 128 << 10;
And after patch 3 this is now left as "unsigned int"?
Good catch. :-)
+static bool debugtrace_per_cpu;
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 ( !strncmp(s, "cpu:", 4) )
+ {
+ debugtrace_per_cpu = true;
+ s += 4;
+ }
+ debugtrace_bytes = parse_size_and_unit(s, NULL);
Stray double blank.
Yes. Will remove one.
+ return 0;
+}
+custom_param("debugtrace", debugtrace_parse_param);
+
+static void debugtrace_dump_buffer(struct debugtrace_data_s *data,
+ const char *which)
{
- if ( !debtr_data || !debugtrace_used )
+ if ( !data )
return;
- printk("debugtrace_dump() starting\n");
+ printk("debugtrace_dump() %s buffer starting\n", which);
/* Print oldest portion of the ring. */
- ASSERT(debtr_data->buf[debtr_data->bytes - 1] == 0);
- if ( debtr_data->buf[debtr_data->prd] != '\0' )
- console_serial_puts(&debtr_data->buf[debtr_data->prd],
- debtr_data->bytes - debtr_data->prd - 1);
+ ASSERT(data->buf[data->bytes - 1] == 0);
+ if ( data->buf[data->prd] != '\0' )
+ console_serial_puts(&data->buf[data->prd], data->bytes - data->prd -
1);
Seeing this getting changed yet another time I now really wonder if
this nul termination is really still needed now that a size is being
passed into the actual output function. If you got rid of this in an
early prereq patch, the subsequent (to that one) ones would shrink.
Yes.
Furthermore I can't help thinking that said change to pass the size
into the actual output functions actually broke the logic here: By
memset()-ing the buffer to zero, output on a subsequent invocation
would have been suitably truncated (in fact, until prd had wrapped,
I think it would have got truncated more than intended). Julien,
can you please look into this apparent regression?
I can do that. Resetting prd to 0 when clearing the buffer is
required here.
@@ -156,33 +195,70 @@ static void debugtrace_key(unsigned char key)
debugtrace_toggle();
}
-static int __init debugtrace_init(void)
+static void debugtrace_alloc_buffer(struct debugtrace_data_s **ptr)
{
int order;
- unsigned long kbytes, bytes;
struct debugtrace_data_s *data;
- /* 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;
+ if ( debugtrace_bytes < PAGE_SIZE || *ptr )
Why the check against PAGE_SIZE?
With recaclulating debugtrace_bytes this can be dropped.
+ return;
- order = get_order_from_bytes(bytes);
+ order = get_order_from_bytes(debugtrace_bytes);
data = alloc_xenheap_pages(order, 0);
if ( !data )
- return -ENOMEM;
+ {
+ printk("failed to allocate debugtrace buffer\n");
Perhaps better to also indicate which/whose buffer?
Hmm, I'm not sure this is really required. I can add it if you want, but
as a user of debugtrace I'd be only interested in the information
whether I can expect all trace entries to be seen or not.
+ return;
+ }
+
+ memset(data, '\0', debugtrace_bytes);
+ data->bytes = debugtrace_bytes - sizeof(*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. */
+ switch ( action )
+ {
+ case CPU_UP_PREPARE:
+ debugtrace_alloc_buffer(&per_cpu(debtr_cpu_data, cpu));
+ break;
+ default:
+ break;
There no apparent need for "default:" here; quite possibly this
could be if() instead of switch() anyway.
Fine with me.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
