Re: [PATCH 2/2] mm: introduce PAGE_FLAGS() to make output of page flags better

[Yafang Shao <laoar.shao@gmail.com>]

On Fri, Jan 15, 2021 at 6:15 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 15.01.21 11:10, Yafang Shao wrote:
> > On Fri, Jan 15, 2021 at 4:39 PM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 15.01.21 07:13, Yafang Shao wrote:
> >>> There're totally __NR_PAGEFLAGS page flags, but the type of page->flags is
> >>> unsigned long, that makes the value of page->flags a little misleading when
> >>> it is printed to the user. We'd better print the real pages flags, instead
> >>> of the whole 64bits including the random values in the useless high bits.
> >>
> >> No, these are *not* random values. They include the nid, zid, and
> >> section_nr - which are helpful to have at hand when debugging, or
> >> detecting that something might be messed up there.
> >>
> >
> > Thanks for the explanation. I just noticed the page-flags layout in
> > page-flags-layout.h.
> >
> >>>
> >>> There're two choices to achieve that, one of which is clear the useless
> >>
> >> Again, not useless.
> >>
> >>> high bits when we initlize the page->flags, the other is don't print the
> >>> high bits when it is showed to the user. The latter one is better because
> >>> it is in the slow path and the performance won't be impacted.
> >>>
> >>> Before that change, the output is,
> >>> [ 8846.517809] INFO: Slab 0x00000000f42a2c60 objects=33 used=3 fp=0x0000000060d32ca8 flags=0x17ffffc0010200(slab|head)
> >>>
> >>> After that change, the output is,
> >>> [ 8843.757770] INFO: Slab 0x00000000f0e98335 objects=33 used=3 fp=0x00000000b643c7d8 flags=0x10200(slab|head)
> >>>
> >>
> >> Nack to the current approach. If you're going to strip this information,
> >> you should expose it differently. E.g., printing page_zonenum() or
> >> page_to_nid(). But still, then we might lose valuable information of
> >> bits stored in there that shouldn't have been set.
> >>
> >
> > How about changing the implementation of pGp in printk() ?
> > In the new implementation of pGp we can dump the full information of
> > page->flags, rather than the flag's name only.
> >
> > For example,
> > 0xXXXXXXXX(node n, nid n, ..., slab|head)
> >
> > That will make it easier to understand, as it is not easy to look into
> > the detail of page-flags layout.
>
> Not completely opposed to this. Just keep in mind that the information
> stored in the high bits differs per configuration. See
> include/linux/page-flags-layout.h
>
> So to dump reliably, you would have to consider all different flavors,
> looking at SECTIONS_WIDTH, NODES_WIDTH, ZONES_WIDTH, LAST_CPUPID_WIDTH,
> KASAN_TAG_WIDTH
>
> Might be helpful for other users as well, like in mm/memory-failure.c
>

Thanks for the suggestion.
I will think about it.

-- 
Thanks
Yafang