Re: [PATCH] block,blkcg: use __GFP_NOWARN for best-effort allocations in blkcg
From: Michal Hocko
Date: Tue Nov 29 2016 - 02:25:21 EST
On Mon 28-11-16 12:19:07, Tejun Heo wrote:
> On Wed, Nov 23, 2016 at 09:50:12AM +0100, Vlastimil Babka wrote:
> > > You'd certainly _hope_ that atomic allocations either have fallbacks
> > > or are harmless if they fail, but I'd still rather see that
> > > __GFP_NOWARN just to make that very much explicit.
> > A global change to GFP_NOWAIT would of course mean that we should audit its
> > users (there don't seem to be many), whether they are using it consciously
> > and should not rather be using GFP_ATOMIC.
> A while ago, I thought about something like, say, GFP_MAYBE which is
> combination of NOWAIT and NOWARN but couldn't really come up with
> scenarios where one would want to use NOWAIT w/o NOWARN. If an
> allocation is important enough to warn the user of its failure, it
> better be dipping into the atomic reserve pool; otherwise, it doesn't
> make sense to make noise.
I do not think we really need a new flag for that and fully agree that
GFP_NOWAIT warning about failure is rarely, if ever, useful.
Historically we didn't use to distinguish atomic (with access to
reserves) allocations from those which just do not want to trigger the
reclaim resp. to sleep (aka optimistic allocation requests). But this
has changed so I guess we can really do the following
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index f8041f9de31e..a53b5187b4da 100644
@@ -246,7 +246,7 @@ struct vm_area_struct;
#define GFP_ATOMIC (__GFP_HIGH|__GFP_ATOMIC|__GFP_KSWAPD_RECLAIM)
#define GFP_KERNEL (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
#define GFP_KERNEL_ACCOUNT (GFP_KERNEL | __GFP_ACCOUNT)
-#define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM)
+#define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM|__GFP_NOWARN)
#define GFP_NOIO (__GFP_RECLAIM)
#define GFP_NOFS (__GFP_RECLAIM | __GFP_IO)
#define GFP_TEMPORARY (__GFP_RECLAIM | __GFP_IO | __GFP_FS | \
this will not catch users who are doing gfp & ~__GFP_DIRECT_RECLAIM but
I would rather not make warn_alloc() even more cluttered with checks.