On 05.09.19 12:28, Jan Beulich wrote:
On 04.09.2019 15:46, Juergen Gross wrote:
@@ -25,33 +26,63 @@ 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 = 128 << 10;
+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 ( !debugtrace_used )
+ if ( !strncmp(s, "cpu:", 4) )
+ {
+ debugtrace_per_cpu = true;
+ s += 4;
+ }
+ debugtrace_bytes = parse_size_and_unit(s, NULL);
I'm afraid you can't pass NULL here, or a trailing % will be at best
ambiguous. In fact, the latest with the addition of % support to the
function I can't see (anymore) how calling the function with a NULL
2nd arg can be a good idea at all.
NP to change that.
+static void debugtrace_dump_worker(void)
+{
+ unsigned int cpu;
+ char buf[16];
+
+ if ( !debugtrace_used )
+ return;
+
+ debugtrace_dump_buffer(dt_data, "global");
+
+ for ( cpu = 0; cpu < nr_cpu_ids; cpu++ )
+ {
+ snprintf(buf, sizeof(buf), "cpu %u", cpu);
+ debugtrace_dump_buffer(per_cpu(dt_cpu_data, cpu), buf);
+ }
I think it would be nice if buf[]'s scope was just the for() body.
Okay.
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;
char cntbuf[24];
va_list args;
unsigned long flags;
unsigned int nr;
+ struct debugtrace_data *data;
+ unsigned int cpu;
- if ( !dt_data )
+ data = debugtrace_per_cpu ? this_cpu(dt_cpu_data) : dt_data;
+ cpu = debugtrace_per_cpu ? smp_processor_id() : 0;
You use "cpu" only ...
@@ -131,16 +169,17 @@ void debugtrace_printk(const char *fmt, ...)
}
else
{
- if ( strcmp(buf, last_buf) )
+ if ( strcmp(buf, last_buf) || cpu != last_cpu )
{
- dt_data->prd_last = dt_data->prd;
+ data->prd_last = data->prd;
last_count = ++count;
+ last_cpu = cpu;
safe_strcpy(last_buf, buf);
snprintf(cntbuf, sizeof(cntbuf), "%u ", count);
}
else
{
- dt_data->prd = dt_data->prd_last;
+ data->prd = data->prd_last;
snprintf(cntbuf, sizeof(cntbuf), "%u-%u ", last_count, ++count);
}
debugtrace_add_to_buf(cntbuf);
.. in this scope, so perhaps worth latching (and declaring) it only
inside the first "else" above?
Fine with me.
@@ -155,34 +194,70 @@ 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, bytes;
struct debugtrace_data *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 || *ptr )
+ return;
- order = get_order_from_bytes(bytes);
+ order = get_order_from_bytes(debugtrace_bytes);
data = alloc_xenheap_pages(order, 0);
if ( !data )
- return -ENOMEM;
+ {
+ if ( debugtrace_per_cpu )
+ printk("failed to allocate debugtrace buffer for cpu %u\n", cpu);
Could I talk you into using the more common "CPU%u: ..." layout here?
Sure.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
