[Crash-utility] [PATCH] display MCNT and PRIVATE when using kmem -p
qiaonuohan
qiaonuohan at cn.fujitsu.com
Tue Jan 10 02:53:22 UTC 2012
Hello Dave,
Glab to hear the capability is desirable. I will start to implement this
soon.
At 2012-1-9 23:50, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> Hello Dave,
>>
>> I think it is necessary to see what is hided in the union where
>> _mapcount lies. However, as you said, it's hard to pick which fields are
>> more important than others when adding new items to "kmem -p". So I
>> think over using struct sub-command to show what I want.
>>
>> What if I change struct sub-command to this:
>>
>> 1. it can refer to anonymous members (e.g., page._mapcount)
>> 2. it can refer to submembers(e.g., page._count.counter)
>> 3. it can output easy-parsing format (using an option to specify), maybe
>> like 'kmem -p'. For example,
>> crash> struct page.flags,_count.counter -..< PAGE_list.txt
>> 1024 0
>> 1024 1
>> 1024 1
>> 1024 1
>>
>> After adding these features to struct sub-command, I guess it is more
>> easier to get information hiding in structs and parsing it. Before
>> implementing, I feel the necessity to ask you for some advices. So what
>> about these features?
>
> That would certainly be useful. The problem in getting that to work
> would be twofold:
>
> (1) handling a "member" request that has a "." in it.
>
> crash> struct page._count.counter ffffea0000000400
> struct: invalid format: page._count.counter
> crash>
>
> (2) getting a successful return value from the call to arg_to_datatype()
> in cmd_datatype_common().
>
> crash> struct page.private ffffea0000000400
> struct: invalid data structure reference: page.private
> crash>
>
> And both of the above would require getting the get_member_data()
> function in gdb-7.3.1/gdb/symtab.c to dig out the information for
> anonymous members, which it currently does not do.
>
> In this part of get_member_data(), the requested member name string is
> searched for:
>
> for (i = 0; i< nfields; i++) {
> if (STREQ(req->member, nextfield->name)) {
> req->member_offset = nextfield->loc.bitpos;
> req->member_length = TYPE_LENGTH(nextfield->type);
> req->member_typecode = TYPE_CODE(nextfield->type);
> if ((req->member_typecode == TYPE_CODE_TYPEDEF)&&
> (typedef_type = check_typedef(nextfield->type)))
> req->member_length = TYPE_LENGTH(typedef_type);
> return;
> }
> nextfield++;
> }
>
> When get_member_data() walks through the page stucture, it does find the
> anonymous members above, but nextfield->name points to a NULL name string.
>
> That seems to be related to gdb's behavior when asking it to simply
> print a page structure, which is requested with a "ptype struct page"
> gdb command:
>
> crash> struct page
> struct page {
> long unsigned int flags;
> struct address_space *mapping;
> struct {
> union {...};
> union {...};
> };
> struct list_head lru;
> union {
> long unsigned int private;
> spinlock_t ptl;
> struct kmem_cache *slab;
> struct page *first_page;
> };
> }
> SIZE: 64
> crash>
>
> I don't know how to get gdb to display the "full" structure declaration?
>
> Anyway, when given an internal request to display a page structure
> from memory, it does display them:
>
> crash> page ffffea0000000400
> struct page {
> flags = 0,
> mapping = 0x0,
> {
> {
> index = 18446612132314288112,
> freelist = 0xffff880000010ff0
> },
> {
> counters = 4294967168,
> {
> {
> _mapcount = {
> counter = -128
> },
> {
> inuse = 65408,
> objects = 32767,
> frozen = 1
> }
> },
> _count = {
> counter = 0
> }
> }
> }
> },
> lru = {
> next = 0xffffea00000042a0,
> prev = 0xffffea00000005a0
> },
> {
> private = 1,
> ptl = {
> {
> rlock = {
> raw_lock = {
> slock = 1
> }
> }
> }
> },
> slab = 0x1,
> first_page = 0x1
> }
> }
> crash>
>
> And because that works OK, the ANON_MEMBER_OFFSET_REQUEST() macro
> exists to handle cases for required offset_table entries.
>
> Here, if I put a debug printf each time though the member loop
> in get_member_data(), you'll see this:
>
> crash> page.slab ffffea0000000400
> page -> flags
> page -> mapping
> page ->
> page -> lru
> page ->
> struct: invalid data structure reference: page.slab
> crash>
>
> which again, reflects what happens when a page struct is printed:
>
> crash> struct page
> struct page {
> long unsigned int flags;
> struct address_space *mapping;
> struct {
> union {...};
> union {...};
> };
> struct list_head lru;
> union {
> long unsigned int private;
> spinlock_t ptl;
> struct kmem_cache *slab;
> struct page *first_page;
> };
> }
> SIZE: 64
> crash>
>
> So, anyway, I would presume that perhaps something could be applied to
> get_member_data() to and check out the fields with NULL name strings, i.e.:
>
> for (i = 0; i< nfields; i++) {
> if (STREQ(req->member, nextfield->name)) {
> req->member_offset = nextfield->loc.bitpos;
> req->member_length = TYPE_LENGTH(nextfield->type);
> req->member_typecode = TYPE_CODE(nextfield->type);
> if ((req->member_typecode == TYPE_CODE_TYPEDEF)&&
> (typedef_type = check_typedef(nextfield->type)))
> req->member_length = TYPE_LENGTH(typedef_type);
> return;
> }
> + if (strlen(nextfield->name) == 0) {
> + ...
> + }
> nextfield++;
> }
>
> And that section would have to somehow deal with members that are part
> of anonymous struct members (like "private"), as well as with anonymous
> members that are expressed with a "." in them (like "_mapcount.counter).
>
> I don't expect it will be very easy to accomplish. But it would be a
> desirable capability, so please be my guest!
>
> Thanks,
> Dave
>
>
>> At 2012-1-6 3:37, Dave Anderson wrote:
>>> I appreciate the effort, but I'm not sure whether it's worth changing
>>> it at this point, or whether it could be accomplished in a different
>>> manner.
>>>
>>> The primary purpose for "kmem -p" is to show the page structure
>>> address associated with each physical address in the system -- along
>>> with "basic information about each page". It's had those basic
>>> fields in it forever -- which BTW, fit into 80 columns. I prefer not
>>> to have command output exceed 80 columns unless it is impossible to
>>> predict the size of an output field.
>>>
>>> Anyway, the page structure definition keeps changing over time, more
>>> specifically the embedded anonymous structures contained within it, and
>>> the fields within the anonymous structs have multiple meanings. With
>>> your patch, the output becomes cluttered and hard to understand, especially
>>> due to the strange values that can be seen in the MCNT column when it's
>>> not a counter value, but rather a slab-page construct:
>>>
>>> union {
>>> atomic_t _mapcount;
>>>
>>> struct {
>>> unsigned inuse:16;
>>> unsigned objects:15;
>>> unsigned frozen:1;
>>> };
>>> };
>>>
>>> And so it's hard to pick which fields are more important than
>>> others,
>>> because it pretty much depends upon what's being debugged. You
>>> have
>>> picked the private field (which can have numerous meanings), but
>>> for
>>> example, there have been times in the past where I wanted to see
>>> the
>>> lru list_head contents.
>>>
>>> That all being said, your patch does have merit, but I wonder if
>>> there
>>> could be an alternate way of selecting or filtering what fields are
>>> displayed?
>>
>>
>> --
>> --
>> Regards
>> Qiao Nuohan
>>
>> --
>> Crash-utility mailing list
>> Crash-utility at redhat.com
>> https://www.redhat.com/mailman/listinfo/crash-utility
>>
>
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>
>
--
--
Regards
Qiao Nuohan
More information about the Crash-utility
mailing list