[PATCH] mm/slub: check pfmemalloc_match in slab_alloc_node fastpath

[PATCH] mm/slub: check pfmemalloc_match in slab_alloc_node fastpath

[Ohhoon Kwon]

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);
-- 
2.25.1

Re: [PATCH] mm/slub: check pfmemalloc_match in slab_alloc_node fastpath

[Hyeonggon Yoo]

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

Re: [PATCH] mm/slub: check pfmemalloc_match in slab_alloc_node fastpath

[Ohoon Kwon]

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