Re: [PATCH v4 03/15] xenctx: Add -m (--multiple_pages) option to output larger stack

[Don Slutz <dslutz@verizon.com>]

On 03/19/14 11:34, George Dunlap wrote:
> On 03/18/2014 10:15 PM, Don Slutz wrote:
>> From: Don Slutz <Don@CloudSwitch.com>
>>
>> Important: This is the stack size to display not the configured
>> stack size.
>>
>> Using pictures (for a 3 page configured system):
>>
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>> SP --> |                  |
>>         |                  |
>>         +------------------+
>
> Sorry, what is this a picture of?  I can't make any sense out of it.  
> Shouldn't this have only one box, the next have two, and the bottom 
> one have three?
>

This is various pictures of a 3 page stack, and where the SP currently 
is.  Each box is a page.  So here the "stack limit" is the end of 1 
page.  In the sense of how much stack is used, you are right it is 1, 2 
and then 3.  This tracks with the value passed for "-m" (see next line).


>>
>> Display using "-m 1" since the used stack pages is 1.
>>
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         +------------------+
>>         |                  |
>> SP --> |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         +------------------+
>>

Here the stack limit is end of  page + 1 full page.

>> Display using "-m 2" since the used stack pages is 2.
>>
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>> SP --> |                  |
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         +------------------+
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         |                  |
>>         +------------------+
>>
>> Display using "-m 3" since the used stack pages is 3.
>>

Here the stack limit is end of  page + 2 full pages.


>> Signed-off-by: Don Slutz <Don@CloudSwitch.com>
>> ---
>> v4 Converted from xenctx: Add -2 (--two-pages) option to switch stack 
>> size to 8KiB
>>
>>   tools/xentrace/xenctx.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
>> index 42a47f3..6da38cc 100644
>> --- a/tools/xentrace/xenctx.c
>> +++ b/tools/xentrace/xenctx.c
>> @@ -35,6 +35,7 @@ static struct xenctx {
>>       int frame_ptrs;
>>       int stack_trace;
>>       int disp_all;
>> +    int multiple_pages;
>>       int all_vcpus;
>>       int self_paused;
>>       xc_dominfo_t dominfo;
>> @@ -664,6 +665,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.multiple_pages > 1 )
>> +        stack_limit += (xenctx.multiple_pages - 1) * XC_PAGE_SIZE;
>>       printf("\n");
>>       printf("Stack:\n");
>>       for (i=1; i<5 && stack < stack_limit; i++) {
>> @@ -834,18 +837,21 @@ static void usage(void)
>>           kernel_start);
>>       printf("  -a, --all          display more registers\n");
>>       printf("  -C, --all-vcpus    print info for all vcpus\n");
>> +    printf("  -m PAGES, --multiple-pages=PAGES\n");
>> +    printf("                     assume the kernel was compiled with 
>> PAGES (default 1) of stacks.\n");
>>   }

Opps, this needs fixing.


>>     int main(int argc, char **argv)
>>   {
>>       int ch;
>>       int ret;
>> -    static const char *sopts = "fs:hak:SC";
>> +    static const char *sopts = "fs:hak:SCm:";
>>       static const struct option lopts[] = {
>>           {"stack-trace", 0, NULL, 'S'},
>>           {"symbol-table", 1, NULL, 's'},
>>           {"frame-pointers", 0, NULL, 'f'},
>>           {"kernel-start", 1, NULL, 'k'},
>> +        {"multiple-pages", 0, NULL, 'm'},
>
> I think I would call the long option "kernel-stack-pages" or something 
> like that.  "Multiple pages" doesn't really convey much meaning.  -m 
> is probably a fine short option, but -n might be more memorable.
>

The issue with "kernel-stack-pages" is that it leads to configured 
kernel stack pages (which for the pictures above is 3).  3 is most 
likely not the number to use here.

A big part of this is that how a "kernel" knows where it is in the stack 
can be simple like for a 2 page stack, 1 page is odd, 2nd page is even.  
(3 pages is most likely more complex, but fence page(s) may help here.)

Maybe stack-limit-in-pages is better?

    -Don Slutz

>  -George
>