[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/3] xen: move debugtrace coding to common/debugtrace.c


  • To: Juergen Gross <JGross@xxxxxxxx>
  • From: Jan Beulich <JBeulich@xxxxxxxx>
  • Date: Mon, 29 Jul 2019 12:45:00 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=suse.com;dmarc=pass action=none header.from=suse.com;dkim=pass header.d=suse.com;arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=zQvpUfiFu8Lp+tP4FDfTmWGOCvMdyLYZttse5modv2U=; b=Aihqi6jNVlVNU+/iaGUCe4IcHTZfQ7pKkOLuRCZP+VO0luRjV0yyKC4SIyrn3Xb53bPyuumb4wwXLATmk3c/gIjvSZCjmriOcZqK0pSYrCztyL/F5ADVEil9w2EbW/ISljdqGzDQXqC4q2CzPZSL6N2af/dbs5pbqMt/BBcUOWaArgxSjz5Oq9nFnPE0+LQO4I8sknH8BufbXL5xEcHxfu/3qIIDp8kkIA3ROiSvt2Lkp8YgU850dFOqIkRVZBaePF1PyrsfcuHL3hpOFBUa+PCISsV6oNcrlQVg/A83r8aWhwLQW0CNrrRn9OyLF3aI+V8Nc/2sBpVP7Xu3/w39xg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NBD6K83pC/w/j5RLo0YREqZeZ3zgvFu16/RRJHgV2T21oW6HndsjwOVA8Jgr/E6ZYNKINK63fGPtlrm0pVeG2sbi8PXV7Y1uyZ+iUbnDUVxt35LIQXgtOMW6/UxGkokCnDwfgiYJd2Wk5DGGREmgI/Gb4eeVlAW/6iFnD+tPdiDwsPjfKjZat32Fn+E50jdMH1DGBFt5wh7jjLgN5JdpP/wVaOBcVmRhZijIBTW2e9WsXzn6UDHahiPc9HUurubiC0JM3aLeTRrySuZmGqUwM9AVoMuNZovlNSPNEjiv3mCUbHqq5IBDhop7G1WDaYlDE5VvRN8xwPXMTnAhKfmgOw==
  • Authentication-results: spf=none (sender IP is ) smtp.mailfrom=JBeulich@xxxxxxxx;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Julien Grall <julien.grall@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 29 Jul 2019 13:10:19 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHVRSA2vhgAAF6AnUuE4HJXiRUWH6bhjJWA
  • Thread-topic: [PATCH 1/3] xen: move debugtrace coding to common/debugtrace.c

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.