9:50 a.m.
----- 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(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility