[Crash-utility] Fix for kmem -p

[Dave Anderson]

David Wilder wrote:

> The struct page has been changed to use an unnamed struct inside a
> union.  Gdb can't determine the offset of members of this unnamed
> structure.  This broke the kmem -p command in crash.    I am pursuing a
> fix from the gdb folks, but I am not convinced they will accept this as
> a bug:)   In this patch, if the offset of page->mapping can't be found
> directly I calculate it.  If the order of the elements in struct page
> should change this will break again, but I cant figure out what else to do.
>
> -------------
> gdb has a problem with unnamed structures in a union.  The offset for
> page->mapping can't be found breaking the "kmem -p" command.  I use a
> alternate way to find the offset.  Ugly, but works.
>
> Signed-off-by: David Wilder <dwilder at us.ibm.com>
> --- memory.c.orig       2006-02-16 16:03:17.000000000 -0800
> +++ memory.c    2006-02-16 17:30:01.000000000 -0800
> @@ -236,6 +236,14 @@
>                 MEMBER_OFFSET_INIT(page_count, "page", "_count");
>         MEMBER_OFFSET_INIT(page_flags, "page", "flags");
>          MEMBER_OFFSET_INIT(page_mapping, "page", "mapping");
> +       /* gdb is having a problem with unnamed structures in a union
> +         * The following "hack" works around this problem. */
> +       if ( !VALID_MEMBER(page_mapping) ){
> +               MEMBER_OFFSET_INIT(page_mapping, "page", "_mapcount");
> +               ASSIGN_OFFSET(page_mapping) =
> +                       MEMBER_OFFSET("page","_mapcount") +
> +                       MEMBER_SIZE("struct page","_mapcount");
> +       }
>          MEMBER_OFFSET_INIT(page_index, "page", "index");
>          MEMBER_OFFSET_INIT(page_buffers, "page", "buffers");
>         MEMBER_OFFSET_INIT(page_lru, "page", "lru");
>
> -------------------------
>

Hi Dave,

This line shouldn't be necessary:

> +        MEMBER_OFFSET_INIT(page_mapping, "page", "_mapcount");

because the subsequent ASSIGN_OFFSET(page_mapping) will just
over-writing what you just put there; remember that MEMBER_OFFSET()
does a "fresh" gdb look-up of structure member each time it's called.
(Using OFFSET(x) alone uses what's permanently stored)

But even that wouldn't work, at least with the 2.6.15 kernel I'm
looking at, because the "private" field comes before the "mapping",
so the calculation would be off by the size of long:

struct page {
        unsigned long flags;            /* Atomic flags, some possibly
                                         * updated asynchronously */
        atomic_t _count;                /* Usage count, see below. */
        atomic_t _mapcount;             /* Count of ptes mapped in mms,
                                         * to show when page is mapped
                                         * & limit reverse map searches.
                                         */
        union {
            struct {
                unsigned long private;          /* Mapping-private opaque data:
                                                 * usually used for buffer_heads
                                                 * if PagePrivate set; used for
                                                 * swp_entry_t if PageSwapCache.
                                                 * When page is free, this
                                                 * indicates order in the buddy
                                                 * system.
                                                 */
                struct address_space *mapping;  /* If low bit clear, points to
                                                 * inode address_space, or NULL.
                                                 * If page mapped as anonymous
                                                 * memory, low bit is set, and
                                                 * it points to anon_vma object:
                                                 * see PAGE_MAPPING_ANON below.
                                                 */
            };
#if NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS
            spinlock_t ptl;
#endif
#ifdef CONFIG_XEN
            struct list_head ballooned;
#endif
        };
...

But aside from all that, I'm wonding whether it's better
to just not bother showing it all.  The OFFSET(page_mapping)
is only used by the mem_map[] dump for "kmem -p".  And since
it's no longer always an address_space pointer, it doesn't
make much sense to show it blindly under the MAPPING heading,
because it could also be one of the other things in the union.
And given that the INDEX field (page.index) is meaningful only
when page->mapping is valid, that value can be pretty much
useless as well. I'm thinking it might be worth creating a new
"kmem -p" header and display that doesn't show MAPPING or
INDEX.  In practical usage, the page-address-to-physical-address
connection is the most important thing, and given that you
can just display the page structure.  Or perhaps just put
a bunch of dashes in the MAPPING field.  Or something like
that.

Dave




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20060217/955a147e/attachment.htm>