[Crash-utility] [PATCH 0/4] To support module percpu symbol

[Toshikazu Nakayama]

I am aware my mistake over previous patch (when check Archive).

+			st->percpu_mod_used += block_size;

This += is bad, = is right.
Please switch patch.

Toshi.

>Hi Dave,
>
>Thank you for welcome, review and testing.
>
>>You've come to the right place... ;-)
>>
>>And your patch submission is fine, although I do prefer that you send the
>>the patches as email attachments instead of inline.  (Although that's my
>>problem because of the Zimbra email client that I use)
>
>Ok, I attach V2 patches. [PATCH 3/4] is updated.
>
>>I tested your patch with two kernel types, one of them the "legacy" version
>>with "pcpu_num_used" and "pcpu_size" symbols (RHEL5), and the second one with
>>current kernels with the "pcpu_reserved_chunk_limit" symbol (RHEL6 and 2.6.37-rc1).
>>
>>Here's what I see, first without, and then with your patch applied.
>
>>In any case, there's no way I can change the st->__per_cpu_end values
>>as you propose -- even if it somehow works with your testing.
>> 
>>Comments?
>
>I did some wrong changes and would understand your mentions.
>
>Updating __per_cpu_end itself makes original UI's corruption.
>And for sure, I want __per_cpu_end to be bumped with your guess.
>
>I fixed these bugs like attachments and tested again,
>of course checked st->__per_cpu_end value advised from you.
>
>One more update is to add is_per_cpu_range().
>Because there are some conditions about percpu range in symbols.c,
>I think it is better to use common function than inline.
>The function returns 0 (not percpu), 1 (kernels) and 2 (modules)
>although callers do not distinguish between 1 and 2 now.
>
>Thanks,
>Toshi
>
>>(1) Testing with a 2.6.18-based RHEL5 kernel:
>>
>>  Without your patch, the stored __per_cpu_start and __per_cpu_end
>>  values are as shown here in the symbol_table_data structure:
>>  
>>    crash> help -s
>>    ...
>>         __per_cpu_start: ffffffff80419000
>>           __per_cpu_end: ffffffff8041f508
>>    ...
>>  
>>  which reflect the beginning and end of the __per_cpu kernel
>>  symbol range:
>>  
>>    crash> sym -l
>>    ...
>>    ffffffff80419000 (A) __per_cpu_start  
>>    ffffffff80419000 (D) per_cpu__init_tss  
>>    ffffffff8041b080 (D) per_cpu__orig_ist  
>>    ffffffff8041b100 (d) per_cpu__idle_state  
>>    ...
>>    ffffffff8041f480 (d) per_cpu__flow_flush_tasklets  
>>    ffffffff8041f4c0 (d) per_cpu__rt_cache_stat  
>>    ffffffff8041f500 (d) per_cpu____icmp_socket  
>>    ffffffff8041f508 (A) __per_cpu_end 
>>  
>>  With your patch applied, the __per_cpu_end gets incremented based 
>>  upon the "pcpu_num_used" and "pcpu_size" contents:
>>  
>>    crash> help -s
>>    ...
>>         __per_cpu_start: ffffffff80419000
>>           __per_cpu_end: ffffffff80427a10
>>    ...
>>    
>>  But that doesn't seem right, because the resultant value
>>  pushes it into the bss symbol range:
>>  
>>    crash> sym ffffffff80427a10
>>    ffffffff80427a10 (B) boot_exception_stacks+6672  
>>    crash> 
>>    
>>    crash> sym -l
>>    ...
>>    ffffffff8041f4c0 (d) per_cpu__rt_cache_stat  
>>    ffffffff8041f500 (d) per_cpu____icmp_socket  
>>    ffffffff8041f508 (A) __per_cpu_end           <-- original value  
>>    ffffffff80420000 (d) .data_nosave  
>>    ffffffff80420000 (A) __nosave_begin  
>>    ffffffff80420000 (D) in_suspend  
>>    ffffffff80421000 (b) .bss  
>>    ffffffff80421000 (A) __bss_start  
>>    ffffffff80421000 (A) __nosave_end  
>>    ffffffff80421000 (B) empty_zero_page  
>>    ffffffff80422000 (B) boot_cpu_stack  
>>    ffffffff80426000 (B) boot_exception_stacks   <-- bumped to ffffffff80427a10
>>    ffffffff8042c000 (B) idt_table  
>>    ffffffff8042d000 (B) boot_delay  
>>    ffffffff8042d008 (B) printk_delay_msec 
>>    ...
>>  
>>(2) Testing with a 2.6.32-based RHEL6 kernel:
>> 
>>  Without your patch, the stored __per_cpu_start and __per_cpu_end
>>  values are as shown here in the symbol_table_data structure:
>>    
>>    crash> help -s
>>    ...
>>        __per_cpu_start: 0
>>          __per_cpu_end: 17358
>>    ...
>>  
>>  which reflect the beginning and end of the kernel's __per_cpu 
>>  symbol range:
>>  
>>    crash> sym -l
>>    0 (D) __per_cpu_start  
>>    0 (V) per_cpu__irq_stack_union  
>>    4000 (V) per_cpu__gdt_page  
>>    5000 (V) per_cpu__exception_stacks  
>>    ...
>>    16940 (V) per_cpu__cpu_tlbstate  
>>    16980 (V) per_cpu__runqueues  
>>    17340 (V) per_cpu__sched_clock_data  
>>    17358 (D) __per_cpu_end  
>>    ffffffff81000000 (T) _text 
>>    ...
>>    
>>  With your patch applied, the st->__per_cpu_end value gets changed 
>>  like so:
>>  
>>    crash> help -s
>>    ...
>>        __per_cpu_start: 0
>>          __per_cpu_end: ffffffff81bf36b8
>>    ...
>>    
>>  But that doesn't seem right, where ffffffff81bf36b8 is the 
>>  "pcpu_reserved_chunk_limit" symbol value:
>>  
>>    crash> sym ffffffff81bf36b8
>>    ffffffff81bf36b8 (b) pcpu_reserved_chunk_limit  
>>    crash>
>>  
>>  I'm guessing that you want it st->__per_cpu_end to be bumped from
>>  0x17358 to 0x19358:
>>  
>>    crash> px pcpu_reserved_chunk_limit
>>    pcpu_reserved_chunk_limit = $4 = 0x19358
>>    crash> 
>>
>>(3) Testing with a generic upstream 2.6.37-rc1 kernel, I see the same 
>>    behaviour as with the RHEL6 kernel.
>>
>>In any case, there's no way I can change the st->__per_cpu_end values
>>as you propose -- even if it somehow works with your testing.
>> 
>>Comments?
>>
>>Dave
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-expand-percpu-area.patch
Type: application/octet-stream
Size: 4047 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20101109/04b5764e/attachment.obj>