On 03.09.19 12:16, Jan Beulich wrote:
On 28.08.2019 10:00, Juergen Gross wrote:
As a preparation for per-cpu buffers do a little refactoring of the
debugtrace data: put the needed buffer admin data into the buffer as
it will be needed for each buffer.
While at it switch debugtrace_send_to_console and debugtrace_used to
bool and delete an empty line.
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
xen/common/debugtrace.c | 62 ++++++++++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 27 deletions(-)
diff --git a/xen/common/debugtrace.c b/xen/common/debugtrace.c
index a2fab0d72c..7a9e4fb99f 100644
--- a/xen/common/debugtrace.c
+++ b/xen/common/debugtrace.c
@@ -15,33 +15,39 @@
#include <xen/watchdog.h>
/* Send output direct to console, or buffer it? */
-static volatile int debugtrace_send_to_console;
+static volatile bool 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;
+struct debugtrace_data_s {
Is the _s suffix really needed for some reason?
No, not really. I can drop it.
+ unsigned long bytes; /* Size of buffer. */
+ unsigned long prd; /* Producer index. */
Seeing that you switch from int to long here - are you really
fancying these buffers to go beyond 4GiB in size?
I didn't want to rule it out on a multi-TB machine. :-)
+ char buf[];
+};
+
+static struct debugtrace_data_s *debtr_data;
How about s/debtr/dt/ or s/debtr/debugtrace/ ?
Yes, dt seems to be fine.
+static unsigned int debugtrace_kilobytes = 128;
Since you touch this anyway, add __initdata? Maybe also move it
next to its integer_param()?
This is modified in patch 4 again, and there I need it in the running
system for support of cpu hotplug with per-cpu buffers.
+static bool debugtrace_used;
static DEFINE_SPINLOCK(debugtrace_lock);
integer_param("debugtrace", debugtrace_kilobytes);
static void debugtrace_dump_worker(void)
{
- if ( (debugtrace_bytes == 0) || !debugtrace_used )
+ if ( !debtr_data || !debugtrace_used )
return;
Why don't you get rid of the left side of the || altogether?
In patch 4 I do. :-)
I can drop it here already.
@@ -165,12 +171,14 @@ static int __init debugtrace_init(void)
return 0;
order = get_order_from_bytes(bytes);
- debugtrace_buf = alloc_xenheap_pages(order, 0);
- ASSERT(debugtrace_buf != NULL);
+ data = alloc_xenheap_pages(order, 0);
+ if ( !data )
+ return -ENOMEM;
- memset(debugtrace_buf, '\0', bytes);
+ memset(data, '\0', bytes);
- debugtrace_bytes = bytes;
+ data->bytes = bytes - sizeof(*data);
+ debtr_data = data;
The allocation may end up being almost twice as big as what gets
actually used this way. Wouldn't it make sense to re-calculate
"bytes" from "order"?
Yes, you are right.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
