[Crash-utility] Yet another kernel change for crash

[Dave Anderson]

Badari Pulavarty wrote:

> On Mon, 2005-11-14 at 15:06 -0500, Dave Anderson wrote:
> > Badari Pulavarty wrote:
> > > Hi Dave,
> > >
> > > It looks like "mm_struct _rss" now got changed.
> > > Now we have "mm_struct _file_rss" instead.
> > >
> > >
> > Hi Badari,
> >
> > This can be handled similarly to the way I suggested re: the
> > kmem_cache
> > issue.  In fact, there already is this in place to deal with the
> > original name
> > change from mm_struct.rss to mm_struct._rss:
> >
> >         MEMBER_OFFSET_INIT(mm_struct_rss, "mm_struct", "rss");
> >         if (!VALID_MEMBER(mm_struct_rss))
> >                 MEMBER_OFFSET_INIT(mm_struct_rss, "mm_struct",
> > "_rss");
> >
> > i.e., itt needs yet another qualifier:
> >
> >        MEMBER_OFFSET_INIT(mm_struct_rss, "mm_struct", "rss");
> >        if (!VALID_MEMBER(mm_struct_rss))
> >                 MEMBER_OFFSET_INIT(mm_struct_rss, "mm_struct",
> > "_rss");
> >        if (!VALID_MEMBER(mm_struct_rss))
> >                 MEMBER_OFFSET_INIT(mm_struct_rss, "mm_struct",
> > "_file_rss");
> >
>
> I already made this change and its working fine. Only concern
> reading comments is, now total rss = file_rss + anon_rss.
> I wasn't sure if it was the case earlier or "rss" used to represent
> the whole thing and now got split up.
>

The additional anon_rss bookkeeping is already in place
get_task_mem_usage(), so the change above should
be sufficient.

>
> > Can you check that out?  I'm also (maybe incorrectly) presuming
> > that you were looking into fixing the kmem_cache issue as well?
> > Is that true?
>
> Well, I hacked up kmem_cache stuff to make it work. I need to
> figure out a clean way to make sure we don't break backward
> compatibility. Ofcourse, I could add VALID_MEMBER() every where
> but code looks ugly.
>
> I was thinking of adding something like ..
>
> MEMBER_OFFSET_INIT_STRUCTS(kmem_cache_s_name, "kmem_cache_s",
>                 "kmem_cache", "name");
>
> Which checks first structure and if INVALID_MEMBER() uses
> the second structure.
>
> What do you think ?
>

In other words, collect all the ugliness in one place?  I suppose
you could do something like that, but you have to check all of the places
the "non-minus-one" kmem_cache_s offsets, sizes, and array lengths
get initialized.  I don't have a problem with adding VALID_MEMBER()
checks in those eleven (?) locations -- it's certainly not the first time
it's been done.  And doing it that way shouldn't break backwards
compatibility, because you're only adding the new code when the
member is invalid.

Thanks,
  Dave