Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220409160224.611353-1-ohkwon1043@gmail.com> <YlKRVOgAFqdgtxDu@hyeyoo>
In-Reply-To: <YlKRVOgAFqdgtxDu@hyeyoo>
From:   Ohoon Kwon <ohkwon1043@gmail.com>
Date:   Mon, 11 Apr 2022 01:13:11 +0900
Message-ID: <CAE_YftXpxco35Z2GNq=-DCtwzoq8dq_z7qrp8vCdrs7ePY3gLw@mail.gmail.com>
Subject: Re: [PATCH] mm/slub: check pfmemalloc_match in slab_alloc_node fastpath
To:     Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc:     Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        JaeSang Yoo <jsyoo5b@gmail.com>,
        Wonhyuk Yang <vvghjk1234@gmail.com>,
        Jiyoup Kim <lakroforce@gmail.com>,
        Donghyeok Kim <dthex5d@gmail.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Oh I was not aware of those histories.
I checked the commits, and it seems it is better to leave it
optimized(as it is now).

Thanks for your help.
Ohhoon Kwon.


On Sun, Apr 10, 2022 at 5:12 PM Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote:
>
> On Sun, Apr 10, 2022 at 01:02:23AM +0900, Ohhoon Kwon wrote:
> > If current alloc context does not have __GFP_MEMALLOC in its gfpflags,
> > then slab objects that were previously created with __GFP_MEMALLOC
> > should not be given.
> >
> > This criteria is well kept in slab alloc slowpath:
> > When gfpflags does not contain __GFP_MEMALLOC but if per-cpu slab page
> > was allocated with __GFP_MEMALLOC, then allocator first deactivates
> > per-cpu slab page and then again allocates new slab page with the
> > current context's gfpflags.
> >
> > However, this criteria is not checked in fastpath.
> > It should also be checked in the fastpath, too.
> >
> > Signed-off-by: Ohhoon Kwon <ohkwon1043@gmail.com>
> > ---
> >  mm/slub.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 74d92aa4a3a2..c77cd548e106 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -3179,7 +3179,8 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s, struct list_l
> >        * there is a suitable cpu freelist.
> >        */
> >       if (IS_ENABLED(CONFIG_PREEMPT_RT) ||
> > -         unlikely(!object || !slab || !node_match(slab, node))) {
> > +         unlikely(!object || !slab || !node_match(slab, node) ||
> > +                     !pfmemalloc_match(slab, gfpflags))) {
> >               object = __slab_alloc(s, gfpflags, node, addr, c);
> >       } else {
> >               void *next_object = get_freepointer_safe(s, object);
>
> The missing pfmemalloc check in fastpath was intended.
>
> pfmemalloc check in fast did exist in Mel's commit 072bb0aa5e0629 ("mm:
> sl[au]b: add knowledge of PFMEMALLOC reserve pages").
>
> But later removed by Christoph's commit 5091b74a95d4 ("mm: slub: optimise
> the SLUB fast path to avoid pfmemalloc checks").
>
> Any thoughts?
>
> Thanks!
>
> --
> Thanks,
> Hyeonggon