On 28.07.2019 10:40, Juergen Gross wrote:
> @@ -1155,178 +1155,6 @@ int printk_ratelimit(void)
> return __printk_ratelimit(printk_ratelimit_ms, printk_ratelimit_burst);
> }
>
> -/*
> - * **************************************************************
> - * *************** Serial console ring buffer *******************
> - * **************************************************************
> - */
I acknowledge that this comment has at best been displaced from what
it would properly belong to, so is indeed perhaps best dropped. But ...
> -#ifdef CONFIG_DEBUG_TRACE
> -
> -/* 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 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. */
> - ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
> - sercon_puts(&debugtrace_buf[debugtrace_prd]);
> -
> - /* Print youngest portion of the ring. */
> - debugtrace_buf[debugtrace_prd] = '\0';
> - sercon_puts(&debugtrace_buf[0]);
> -
> - memset(debugtrace_buf, '\0', debugtrace_bytes);
> -
> - 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;
> - /* Always leave a nul byte at the end of the buffer. */
> - if ( debugtrace_prd == (debugtrace_bytes - 1) )
> - debugtrace_prd = 0;
> - }
> -}
> -
> -void debugtrace_printk(const char *fmt, ...)
> -{
> - static char buf[1024], last_buf[1024];
> - static unsigned int count, last_count, last_prd;
> -
> - char cntbuf[24];
> - va_list args;
> - unsigned long flags;
> -
> - if ( debugtrace_bytes == 0 )
> - return;
> -
> - debugtrace_used = 1;
> -
> - spin_lock_irqsave(&debugtrace_lock, flags);
> -
> - ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
> -
> - va_start(args, fmt);
> - vsnprintf(buf, sizeof(buf), fmt, args);
> - va_end(args);
> -
> - if ( debugtrace_send_to_console )
> - {
> - snprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
> - serial_puts(sercon_handle, cntbuf);
> - serial_puts(sercon_handle, buf);
> - }
> - else
> - {
> - if ( strcmp(buf, last_buf) )
> - {
> - last_prd = debugtrace_prd;
> - last_count = ++count;
> - safe_strcpy(last_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 ***************
... what about this one? There's only panic() between it and the next
such comment, and I don't think the "Debugging/tracing" part of it
are applicable (anymore).
> --- a/xen/include/xen/console.h
> +++ b/xen/include/xen/console.h
> @@ -48,4 +48,8 @@ int console_resume(void);
>
> extern int8_t opt_console_xen;
>
> +/* Issue string via serial line. */
> +extern int sercon_handle;
> +void sercon_puts(const char *s);
I guess avoiding their exposure was one of the reasons the debug trace
code lived in the place you move it from. I'm unconvinced non-console
code is actually supposed to make use of either, but I'm not opposed
enough to nak the change. I don't think though the comment fits well
with the variable declaration.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
