----- 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
