From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
To: "HORIGUCHI NAOYA(堀口 直也)" <naoya.horiguchi@nec.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
Andrew Morton <akpm@linux-foundation.org>,
Christoph Lameter <cl@linux.com>,
Pekka Enberg <penberg@kernel.org>,
David Rientjes <rientjes@google.com>,
Vlastimil Babka <vbabka@suse.cz>,
Miaohe Lin <linmiaohe@huawei.com>,
Matthew Wilcox <willy@infradead.org>,
Minchan Kim <minchan@kernel.org>,
Mel Gorman <mgorman@techsingularity.net>,
Andrea Arcangeli <aarcange@redhat.com>,
Dan Williams <dan.j.williams@intel.com>,
Hugh Dickins <hughd@google.com>,
Muchun Song <songmuchun@bytedance.com>,
David Hildenbrand <david@redhat.com>
Subject: Re: [RFC v2 1/3] mm: move PG_slab flag to page_type
Date: Wed, 9 Nov 2022 14:45:26 +0900 [thread overview]
Message-ID: <Y2s+dnBsHAJu19ob@hyeyoo> (raw)
In-Reply-To: <20221108053923.GA481964@hori.linux.bs1.fc.nec.co.jp>
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
next prev parent reply other threads:[~2022-11-09 5:45 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-06 14:03 [RFC v2 0/3] move PG_slab to page_type Hyeonggon Yoo
2022-11-06 14:03 ` [RFC v2 1/3] mm: move PG_slab flag " Hyeonggon Yoo
2022-11-08 5:39 ` HORIGUCHI NAOYA(堀口 直也)
2022-11-09 5:45 ` Hyeonggon Yoo [this message]
2022-11-06 14:03 ` [RFC v2 2/3] mm: introduce show_page_types() to provide human-readable page_type Hyeonggon Yoo
2022-11-06 18:23 ` Steven Rostedt
2022-11-09 6:19 ` Hyeonggon Yoo
2022-11-06 14:03 ` [RFC v2 3/3] mm, printk: introduce new format %pGt for page_type Hyeonggon Yoo
2022-11-06 18:04 ` Joe Perches
2022-11-09 6:14 ` Hyeonggon Yoo
2022-11-09 8:13 ` Petr Mladek
2022-11-07 11:18 ` Andy Shevchenko
2022-11-07 14:20 ` Petr Mladek
2022-11-07 14:41 ` Andy Shevchenko
2022-11-09 6:04 ` Hyeonggon Yoo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y2s+dnBsHAJu19ob@hyeyoo \
--to=42.hyeyoo@gmail.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=dan.j.williams@intel.com \
--cc=david@redhat.com \
--cc=hughd@google.com \
--cc=linmiaohe@huawei.com \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=minchan@kernel.org \
--cc=naoya.horiguchi@nec.com \
--cc=penberg@kernel.org \
--cc=rientjes@google.com \
--cc=songmuchun@bytedance.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).