* [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set @ 2018-04-18 14:45 Christopher Lameter 2018-04-18 15:05 ` Mikulas Patocka 2018-04-19 11:00 ` Michal Hocko 0 siblings, 2 replies; 8+ messages in thread From: Christopher Lameter @ 2018-04-18 14:45 UTC (permalink / raw) To: Vlastimil Babka Cc: Mikulas Patocka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, David Rientjes, Joonsoo Kim, Andrew Morton, linux-kernel 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 <cl@linux.com> 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; /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set 2018-04-18 14:45 [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set Christopher Lameter @ 2018-04-18 15:05 ` Mikulas Patocka 2018-04-18 15:11 ` Christopher Lameter 2018-04-18 18:49 ` David Rientjes 2018-04-19 11:00 ` Michal Hocko 1 sibling, 2 replies; 8+ messages in thread From: Mikulas Patocka @ 2018-04-18 15:05 UTC (permalink / raw) To: Christopher Lameter Cc: Vlastimil Babka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, David Rientjes, Joonsoo Kim, Andrew Morton, linux-kernel 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 <cl@linux.com> > > 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; /* ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set 2018-04-18 15:05 ` Mikulas Patocka @ 2018-04-18 15:11 ` Christopher Lameter 2018-04-18 18:49 ` David Rientjes 1 sibling, 0 replies; 8+ messages in thread From: Christopher Lameter @ 2018-04-18 15:11 UTC (permalink / raw) To: Mikulas Patocka Cc: Vlastimil Babka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, David Rientjes, Joonsoo Kim, Andrew Morton, linux-kernel 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 <cl@linux.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set 2018-04-18 15:05 ` Mikulas Patocka 2018-04-18 15:11 ` Christopher Lameter @ 2018-04-18 18:49 ` David Rientjes 1 sibling, 0 replies; 8+ messages in thread From: David Rientjes @ 2018-04-18 18:49 UTC (permalink / raw) To: Mikulas Patocka Cc: Christopher Lameter, Vlastimil Babka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, Joonsoo Kim, Andrew Morton, linux-kernel 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 <cl@linux.com> > > > > 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)? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set 2018-04-18 14:45 [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set Christopher Lameter 2018-04-18 15:05 ` Mikulas Patocka @ 2018-04-19 11:00 ` Michal Hocko 2018-04-20 14:53 ` Christopher Lameter 1 sibling, 1 reply; 8+ messages in thread From: Michal Hocko @ 2018-04-19 11:00 UTC (permalink / raw) To: Christopher Lameter Cc: Vlastimil Babka, Mikulas Patocka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, David Rientjes, Joonsoo Kim, Andrew Morton, linux-kernel 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 <cl@linux.com> > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set 2018-04-19 11:00 ` Michal Hocko @ 2018-04-20 14:53 ` Christopher Lameter 2018-04-21 17:02 ` Vlastimil Babka 0 siblings, 1 reply; 8+ messages in thread From: Christopher Lameter @ 2018-04-20 14:53 UTC (permalink / raw) To: Michal Hocko Cc: Vlastimil Babka, Mikulas Patocka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, David Rientjes, Joonsoo Kim, Andrew Morton, linux-kernel 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set 2018-04-20 14:53 ` Christopher Lameter @ 2018-04-21 17:02 ` Vlastimil Babka 2018-04-23 22:41 ` Christopher Lameter 0 siblings, 1 reply; 8+ messages in thread From: Vlastimil Babka @ 2018-04-21 17:02 UTC (permalink / raw) To: Christopher Lameter, Michal Hocko Cc: Mikulas Patocka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, David Rientjes, Joonsoo Kim, Andrew Morton, linux-kernel 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? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set 2018-04-21 17:02 ` Vlastimil Babka @ 2018-04-23 22:41 ` Christopher Lameter 0 siblings, 0 replies; 8+ messages in thread From: Christopher Lameter @ 2018-04-23 22:41 UTC (permalink / raw) To: Vlastimil Babka Cc: Michal Hocko, Mikulas Patocka, Mike Snitzer, Matthew Wilcox, Pekka Enberg, linux-mm, dm-devel, David Rientjes, Joonsoo Kim, Andrew Morton, linux-kernel 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-23 22:41 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-18 14:45 [PATCH] SLUB: Do not fallback to mininum order if __GFP_NORETRY is set Christopher Lameter 2018-04-18 15:05 ` Mikulas Patocka 2018-04-18 15:11 ` Christopher Lameter 2018-04-18 18:49 ` David Rientjes 2018-04-19 11:00 ` Michal Hocko 2018-04-20 14:53 ` Christopher Lameter 2018-04-21 17:02 ` Vlastimil Babka 2018-04-23 22:41 ` Christopher Lameter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.