Re: [Xen-devel] [PATCH v8 03/20] xenctx: Add -n (--display-stack-pages) option to output larger stack

[Don Slutz <dslutz@xxxxxxxxxxx>]

On 04/01/14 09:25, George Dunlap wrote:
> On 04/01/2014 02:19 PM, Ian Campbell wrote:
> > On Thu, 2014-03-27 at 15:05 -0400, Don Slutz wrote:
> > > Important: This is the stack size (also known as stack limit) to
> > > display not the configured stack size.
> > >
> > > Note: use with caution (easy to get garbage).
> > >
> > > Below is a pictures of a configured 3 page stack, and where
> > > the SP currently is.  Each box is a page.
> > >
> > >                              -n 1    -n 2    -n 3
> > >
> > >         +------------------+
> > >         |                  |
> > >         |                  |
> > >         |                  |
> > >         |                  |
> > > SP --> |                  |   *       *       *
> > >         +------------------+             |          |
> > >         |                  |          |          |
> > >         |                  |          |          |
> > >         |                  |          |          |
> > >         |                  |          |          |
> > >         |                  |          *          |
> > >         +------------------+                    |
> > >         |                  |              |
> > >         |                  |              |
> > >         |                  |              |
> > >         |                  |              |
> > >         |                  |              *
> > >         +------------------+
> > >
> > > Display using "-n 3" since the used stack pages is 3.
> > Stacks grow downwards on all of the architectures we support. Perhaps
> > that is why the rest of us find your diagrams so confusing?
I expect so.  I can flip the picture (and add relative address) to help.  I was 
basing
the layout to match the stack output:

Call Trace:
                    [<ffffffff81346898>] io_serial_out+0x18 <--
ffff880032bb1310:   [<ffffffff81346f51>] serial8250_console_putchar+0x31
ffff880032bb1330:   [<ffffffff813428de>] uart_console_write+0x3e
ffff880032bb1338:   [<ffffffff8100bc0e>] apic_timer_interrupt+0xe
ffff880032bb1370:   [<ffffffff813472ad>] serial8250_console_write+0xbd
ffff880032bb13c0:   [<ffffffff8106b8f5>] __call_console_drivers+0x75
ffff880032bb13f0:   [<ffffffff8106b95a>] _call_console_drivers+0x4a
ffff880032bb1410:   [<ffffffff8106be6e>] release_console_sem+0x4e
ffff880032bb1450:   [<ffffffff8106c628>] vprintk+0x248
ffff880032bb14f0:   [<ffffffff814fd363>] printk+0x41
ffff880032bb3f20:   [<ffffffff8100204c>] do_one_initcall+0x3c
ffff880032bb3f50:   [<ffffffff810b0eb1>] sys_init_module+0xe1
ffff880032bb3f80:   [<ffffffff8100b0f2>] system_call_fastpath+0x16

Adding (my) boxes (note not fully to scale):

    
+------------------------------------------------------------------------------+
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                     [<ffffffff81346898>] io_serial_out+0x18 <--           
   |
SP->| ffff880032bb1310:   [<ffffffff81346f51>] serial8250_console_putchar+0x31  
   |
    | ffff880032bb1330:   [<ffffffff813428de>] uart_console_write+0x3e          
   |
    | ffff880032bb1338:   [<ffffffff8100bc0e>] apic_timer_interrupt+0xe         
   |
    | ffff880032bb1370:   [<ffffffff813472ad>] serial8250_console_write+0xbd    
   |
    | ffff880032bb13c0:   [<ffffffff8106b8f5>] __call_console_drivers+0x75      
   |
    | ffff880032bb13f0:   [<ffffffff8106b95a>] _call_console_drivers+0x4a       
   |
    | ffff880032bb1410:   [<ffffffff8106be6e>] release_console_sem+0x4e         
   |
    | ffff880032bb1450:   [<ffffffff8106c628>] vprintk+0x248                    
   |
    | ffff880032bb14f0:   [<ffffffff814fd363>] printk+0x41                      
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    
+------------------------------------------------------------------------------+
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    
+------------------------------------------------------------------------------+
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    |                                                                           
   |
    | ffff880032bb3f20:   [<ffffffff8100204c>] do_one_initcall+0x3c             
   |
    | ffff880032bb3f50:   [<ffffffff810b0eb1>] sys_init_module+0xe1             
   |
    | ffff880032bb3f80:   [<ffffffff8100b0f2>] system_call_fastpath+0x16        
   |
    |                                                                           
   |
    
+------------------------------------------------------------------------------+



> > I drew my example to you the way I did for a reason.
Sorry, I did not notice that.


> > Also, should the other end of the *---* line from SP (the bottom in your
> > diagram above) not be aligned to a page boundary, after all -n works in
> > pages.
I am happy to do it either way.  I think of the page breaks to not be part of 
memory.


> > > @@ -664,6 +667,8 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int 
> > > vcpu, int width)
> > >
> > >       stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE)
> > >                      & ~((guest_word_t) XC_PAGE_SIZE - 1));
> > > +    if ( xenctx.nr_stack_pages > 1 )
> > > +        stack_limit += (xenctx.nr_stack_pages - 1) * XC_PAGE_SIZE;
> > The if here is still redundant.
Will drop.

> > >       printf("\n");
> > >       printf("Stack:\n");
> > >       for (i=1; i<5 && stack < stack_limit; i++) {
> > > @@ -834,18 +839,24 @@ static void usage(void)
> > >           kernel_start);
> > >       printf("  -a, --all          display more registers\n");
> > >       printf("  -C, --all-vcpus    print info for all vcpus\n");
> > > +    printf("  -n PAGES, --display-stack-pages=PAGES\n");
> > > +    printf("                     Display N pages from the stack pointer. (default 
> > > %d)\n",
> > > +           DEFAULT_NR_STACK_PAGES);
> > > +    printf("                     Changes stack limit.  Note: use with caution 
> > > (easy\n");
> > > +    printf("                     to get garbage).\n");
> > Doesn't it go without saying that if you go off the bottom of the stack
> > you will get garbage?
> Only if you know that the tool doesn't know where the end of the stack is.  (Although 
> hopefully the wording, "Display N pages" should give you a hint.)
>
> But what is "changes stack limit" supposed to mean?
Partly it is used in the next patch's usage output:

 -l <lines>, --lines <lines>
                     change the number of lines output for Stack. (default 5)
                     Can be specified as MAX.  Note: Fewer lines will be output
                     if stack limit reached.


And it was the best term I know of to say in a few words:

The output of stack starts at the current SP and stops when the
end of N page(s) is/are reached.

   -Don Slutz



>  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel