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

[David Hildenbrand <david@redhat.com>]

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,

David / dhildenb