Mikulas Patoka wants to ensure that no fallback to lower order happens. I
think __GFP_NORETRY should work correctly in that case too and not fall
back.
Allocating at a smaller order is a retry operation and should not
be attempted.
If the caller does not want retries then respect that.
GFP_NORETRY allows callers to ensure that only maximum order
allocations are attempted.
Signed-off-by: Christoph Lameter <[email protected]>
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c
+++ linux/mm/slub.c
@@ -1598,7 +1598,7 @@ static struct page *allocate_slab(struct
alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
page = alloc_slab_page(s, alloc_gfp, node, oo);
- if (unlikely(!page)) {
+ if (unlikely(!page) && !(flags & __GFP_NORETRY)) {
oo = s->min;
alloc_gfp = flags;
/*
On Wed, 18 Apr 2018, Christopher Lameter wrote:
> Mikulas Patoka wants to ensure that no fallback to lower order happens. I
> think __GFP_NORETRY should work correctly in that case too and not fall
> back.
>
>
>
> Allocating at a smaller order is a retry operation and should not
> be attempted.
>
> If the caller does not want retries then respect that.
>
> GFP_NORETRY allows callers to ensure that only maximum order
> allocations are attempted.
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> Index: linux/mm/slub.c
> ===================================================================
> --- linux.orig/mm/slub.c
> +++ linux/mm/slub.c
> @@ -1598,7 +1598,7 @@ static struct page *allocate_slab(struct
> alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
>
> page = alloc_slab_page(s, alloc_gfp, node, oo);
> - if (unlikely(!page)) {
> + if (unlikely(!page) && !(flags & __GFP_NORETRY)) {
> oo = s->min;
> alloc_gfp = flags;
> /*
No, this would hit NULL pointer dereference if page is NULL and
__GFP_NORETRY is set. You want this:
---
mm/slub.c | 2 ++
1 file changed, 2 insertions(+)
Index: linux-2.6/mm/slub.c
===================================================================
--- linux-2.6.orig/mm/slub.c 2018-04-17 20:58:23.000000000 +0200
+++ linux-2.6/mm/slub.c 2018-04-18 17:04:01.000000000 +0200
@@ -1599,6 +1599,8 @@ static struct page *allocate_slab(struct
page = alloc_slab_page(s, alloc_gfp, node, oo);
if (unlikely(!page)) {
+ if (flags & __GFP_NORETRY)
+ goto out;
oo = s->min;
alloc_gfp = flags;
/*
On Wed, 18 Apr 2018, Mikulas Patocka wrote:
> No, this would hit NULL pointer dereference if page is NULL and
> __GFP_NORETRY is set. You want this:
You are right
Acked-by: Christoph Lameter <[email protected]>
On Wed, 18 Apr 2018, Mikulas Patocka wrote:
> > Mikulas Patoka wants to ensure that no fallback to lower order happens. I
> > think __GFP_NORETRY should work correctly in that case too and not fall
> > back.
> >
> >
> >
> > Allocating at a smaller order is a retry operation and should not
> > be attempted.
> >
> > If the caller does not want retries then respect that.
> >
> > GFP_NORETRY allows callers to ensure that only maximum order
> > allocations are attempted.
> >
> > Signed-off-by: Christoph Lameter <[email protected]>
> >
> > Index: linux/mm/slub.c
> > ===================================================================
> > --- linux.orig/mm/slub.c
> > +++ linux/mm/slub.c
> > @@ -1598,7 +1598,7 @@ static struct page *allocate_slab(struct
> > alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
> >
> > page = alloc_slab_page(s, alloc_gfp, node, oo);
> > - if (unlikely(!page)) {
> > + if (unlikely(!page) && !(flags & __GFP_NORETRY)) {
> > oo = s->min;
> > alloc_gfp = flags;
> > /*
>
> No, this would hit NULL pointer dereference if page is NULL and
> __GFP_NORETRY is set. You want this:
>
> ---
> mm/slub.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> Index: linux-2.6/mm/slub.c
> ===================================================================
> --- linux-2.6.orig/mm/slub.c 2018-04-17 20:58:23.000000000 +0200
> +++ linux-2.6/mm/slub.c 2018-04-18 17:04:01.000000000 +0200
> @@ -1599,6 +1599,8 @@ static struct page *allocate_slab(struct
>
> page = alloc_slab_page(s, alloc_gfp, node, oo);
> if (unlikely(!page)) {
> + if (flags & __GFP_NORETRY)
> + goto out;
> oo = s->min;
> alloc_gfp = flags;
> /*
>
I don't see the connection between the max order, which can be influenced
by userspace with slub_min_objects, slub_min_order, etc, and specifying
__GFP_NORETRY which means try to reclaim and free memory but don't loop.
If I force a slab cache to try a max order of 9 for hugepages as a best
effort, why does __GFP_NORETRY suddenly mean I won't fallback to
oo_order(s->min)?
On Wed 18-04-18 09:45:39, Cristopher Lameter wrote:
> Mikulas Patoka wants to ensure that no fallback to lower order happens. I
> think __GFP_NORETRY should work correctly in that case too and not fall
> back.
Overriding __GFP_NORETRY is just a bad idea. It will make the semantic
of the flag just more confusing. Note there are users who use
__GFP_NORETRY as a way to suppress heavy memory pressure and/or the OOM
killer. You do not want to change the semantic for them.
Besides that the changelog is less than optimal. What is the actual
problem? Why somebody doesn't want a fallback? Is there a configuration
that could prevent the same?
> Allocating at a smaller order is a retry operation and should not
> be attempted.
>
> If the caller does not want retries then respect that.
>
> GFP_NORETRY allows callers to ensure that only maximum order
> allocations are attempted.
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> Index: linux/mm/slub.c
> ===================================================================
> --- linux.orig/mm/slub.c
> +++ linux/mm/slub.c
> @@ -1598,7 +1598,7 @@ static struct page *allocate_slab(struct
> alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
>
> page = alloc_slab_page(s, alloc_gfp, node, oo);
> - if (unlikely(!page)) {
> + if (unlikely(!page) && !(flags & __GFP_NORETRY)) {
> oo = s->min;
> alloc_gfp = flags;
> /*
--
Michal Hocko
SUSE Labs
On Thu, 19 Apr 2018, Michal Hocko wrote:
> Overriding __GFP_NORETRY is just a bad idea. It will make the semantic
> of the flag just more confusing. Note there are users who use
> __GFP_NORETRY as a way to suppress heavy memory pressure and/or the OOM
> killer. You do not want to change the semantic for them.
Redoing the allocation after failing a large order alloc is a retry. I
would say its confusing right now because a retry occurs despite
specifying GFP_NORETRY,
> Besides that the changelog is less than optimal. What is the actual
> problem? Why somebody doesn't want a fallback? Is there a configuration
> that could prevent the same?
The problem is that SLUB does not honor GFP_NORETRY. The semantics of
GFP_NORETRY are not followed.
On 04/20/2018 04:53 PM, Christopher Lameter wrote:
> On Thu, 19 Apr 2018, Michal Hocko wrote:
>
>> Overriding __GFP_NORETRY is just a bad idea. It will make the semantic
>> of the flag just more confusing. Note there are users who use
>> __GFP_NORETRY as a way to suppress heavy memory pressure and/or the OOM
>> killer. You do not want to change the semantic for them.
>
> Redoing the allocation after failing a large order alloc is a retry. I
> would say its confusing right now because a retry occurs despite
> specifying GFP_NORETRY,
>
>> Besides that the changelog is less than optimal. What is the actual
>> problem? Why somebody doesn't want a fallback? Is there a configuration
>> that could prevent the same?
>
> The problem is that SLUB does not honor GFP_NORETRY. The semantics of
> GFP_NORETRY are not followed.
The caller might want SLUB to try hard to get that high-order page that
will minimize memory waste (e.g. 2MB page for 3 640k objects), and
__GFP_NORETRY will kill the effort on allocating that high-order page.
Thus, using __GPF_NORETRY for "please give me a space-optimized object,
or nothing (because I have a fallback that's better than wasting memory,
e.g. by using 1MB page for 640kb object)" is not ideal.
Maybe __GFP_RETRY_MAYFAIL is a better fit? Or perhaps indicate this
situation to SLUB with e.g. __GFP_COMP, although that's rather ugly?
On Sat, 21 Apr 2018, Vlastimil Babka wrote:
> > The problem is that SLUB does not honor GFP_NORETRY. The semantics of
> > GFP_NORETRY are not followed.
>
> The caller might want SLUB to try hard to get that high-order page that
> will minimize memory waste (e.g. 2MB page for 3 640k objects), and
> __GFP_NORETRY will kill the effort on allocating that high-order page.
Well yes since *_NORETRY says that fallbacks are acceptable.
> Thus, using __GPF_NORETRY for "please give me a space-optimized object,
> or nothing (because I have a fallback that's better than wasting memory,
> e.g. by using 1MB page for 640kb object)" is not ideal.
>
> Maybe __GFP_RETRY_MAYFAIL is a better fit? Or perhaps indicate this
> situation to SLUB with e.g. __GFP_COMP, although that's rather ugly?
Yuck. None of that sounds like an intuitive approach.