linux-kernel - Re: [PATCH] block,blkcg: use __GFP_NOWARN for best-effort allocations in blkcg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161129072507.GA31671@dhcp22.suse.cz>
Date:   Tue, 29 Nov 2016 08:25:07 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Tejun Heo <tj@...nel.org>
Cc:     Vlastimil Babka <vbabka@...e.cz>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Jens Axboe <axboe@...nel.dk>, linux-mm <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Marc MERLIN <marc@...lins.org>
Subject: Re: [PATCH] block,blkcg: use __GFP_NOWARN for best-effort
 allocations in blkcg

On Mon 28-11-16 12:19:07, Tejun Heo wrote:
> Hello,
> 
> 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
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -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.
-- 
Michal Hocko
SUSE Labs