On Tue, Nov 08, 2022 at 05:39:24AM +0000, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Sun, Nov 06, 2022 at 11:03:53PM +0900, Hyeonggon Yoo wrote:
> > For now, only SLAB uses _mapcount field as a number of active objects in
> > a slab, and other slab allocators do not use it. As 16 bits are enough
> > for that, use remaining 16 bits of _mapcount as page_type even when
> > SLAB is used. And then move PG_slab flag to page_type.
> >
> > As suggested by Matthew, store number of active objects in negative
> > form and use helper when accessing or modifying it.
> >
> > Note that page_type is always placed in upper 16 bits of _mapcount to
> > avoid confusing normal _mapcount as page_type. As underflow (actually
> > I mean, yeah, overflow) is not a concern anymore, use more lower bits.
> >
> > Add more folio helpers for PAGE_TYPE_OPS() not to break existing
> > slab implementations.
> >
> > Remove PG_slab check from PAGE_FLAGS_CHECK_AT_FREE. buddy will still
> > check if _mapcount is properly set at free.
> >
> > Exclude PG_slab from hwpoison and show_page_flags() for now.
> >
> > Note that with this patch, page_mapped() and folio_mapped() always return
> > false for slab page.
> >
> ...
>
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index 779a426d2cab..9494f47c4cee 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -1145,7 +1145,6 @@ static int me_huge_page(struct page_state *ps, struct page *p)
> > #define mlock (1UL << PG_mlocked)
> > #define lru (1UL << PG_lru)
> > #define head (1UL << PG_head)
> > -#define slab (1UL << PG_slab)
> > #define reserved (1UL << PG_reserved)
> >
> > static struct page_state error_states[] = {
> > @@ -1155,13 +1154,6 @@ static struct page_state error_states[] = {
> > * PG_buddy pages only make a small fraction of all free pages.
> > */
> >
> > - /*
> > - * Could in theory check if slab page is free or if we can drop
> > - * currently unused objects without touching them. But just
> > - * treat it as standard kernel for now.
> > - */
> > - { slab, slab, MF_MSG_SLAB, me_kernel },
> > -
>
> Hi Hyeonggon,
>
Hi Naoya.
> Actually the above part is dead code now and it's harmless to remove this.
> identify_page_state() is never called when handling memory error event
> on a slab page because HWPoisonHandlable() returns false for it.
Oh wasn't aware of it. Thanks. That makes it easier.
> If you remove it, a few other lines using MF_MSG_SLAB will be unneccessary,
> so could you remove them too?
Sure. Will split this to separate patch then.
> As for testing, you can test this case for example like below:
>
> - install page-types (available in tools/vm/page-types.c),
> - show the list of slab pages by "page-types -b slab -Nl" command
> (the first column is PFNs) and choose one PFNs as target,
> - call "page-types -a <PFN> -X" (requires hwpoison-inject module to be loaded)
>
> In my testing server, dmesg shows like below:
>
> [598746.497805] Injecting memory failure at pfn 0x16295b
> [598746.499208] Memory failure: 0x16295b: unhandlable page.
> [598746.500570] Memory failure: 0x16295b: recovery action for unknown page: Ignored
>
> And your patchset seems not to affect this behavior.
Cool! Will test it before sending next version.
Thanks for looking at this :-)
--
Thanks,
Hyeonggon
