linux-kernel - Re: [PATCH 0/3 -v3] GFP

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <201701051950.EAB48947.FFVSHOOQMJtLFO@I-love.SAKURA.ne.jp>
Date:   Thu, 5 Jan 2017 19:50:23 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     mhocko@...nel.org
Cc:     akpm@...ux-foundation.org, hannes@...xchg.org, rientjes@...gle.com,
        mgorman@...e.de, hillf.zj@...baba-inc.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3 -v3] GFP_NOFAIL cleanups

Michal Hocko wrote:
> > > Stop this! Seriously... This is just wasting time...
> > 
> > You are free to ignore me. But
> 
> my last reply in this subthread
> 

OK. You can ignore me; I just won't give my Acked-by: or Reviewed-by: to this series.

My understanding is that we changed to tolerate __GFP_NOFAIL usage because
allocation failure leads to unacceptable side effect (e.g. remounting
read-only, kernel panic) rather than allocation helps reclaiming memory.

  commit 647757197cd34fae ("mm: clarify __GFP_NOFAIL deprecation status")
  commit 277fb5fc177dc467 ("btrfs: use __GFP_NOFAIL in alloc_btrfs_bio")

I don't know whether __GFP_NOFAIL users are using __GFP_NOFAIL based on
whether it helps reclaiming memory rather than whether allocation failure
leads to unacceptable side effect, if we allow access to memory reserves
based on __GFP_NOFAIL.

  commit 7444a072c387a93e ("ext4: replace open coded nofail allocation in ext4_free_blocks()")
  commit adb7ef600cc9d9d1 ("ext4: use __GFP_NOFAIL in ext4_free_blocks()")
  commit c9af28fdd44922a6 ("ext4 crypto: don't let data integrity writebacks fail with ENOMEM")
  commit b32e4482aadfd132 ("fscrypto: don't let data integrity writebacks fail with ENOMEM")
  commit 80c545055dc7c1f7 ("f2fs: use __GFP_NOFAIL to avoid infinite loop")

If __GFP_NOFAIL users are using __GFP_NOFAIL based on whether allocation failure
leads to unacceptable side effect, allowing access to memory reserves based on
__GFP_NOFAIL might not help reclaiming memory; something like scope GFP_NOFS API
will be needed.

Anyway, I suggest merging description update shown below into this series and
getting confirmation from all existing __GFP_NOFAIL users. If all existing
__GFP_NOFAIL users are OK with this series (in other words, informed the risk
caused by this series), I'm also OK with this series.

--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -135,16 +135,24 @@
  * __GFP_REPEAT: Try hard to allocate the memory, but the allocation attempt
  *   _might_ fail.  This depends upon the particular VM implementation.
  *
- * __GFP_NOFAIL: The VM implementation _must_ retry infinitely: the caller
- *   cannot handle allocation failures. New users should be evaluated carefully
- *   (and the flag should be used only when there is no reasonable failure
- *   policy) but it is definitely preferable to use the flag rather than
- *   opencode endless loop around allocator.
- *
- * __GFP_NORETRY: The VM implementation must not retry indefinitely and will
- *   return NULL when direct reclaim and memory compaction have failed to allow
- *   the allocation to succeed.  The OOM killer is not called with the current
- *   implementation.
+ * __GFP_NOFAIL: The VM implementation must not give up even after direct
+ *   reclaim and memory compaction have failed to allow the allocation to
+ *   succeed. Note that since the OOM killer is not called with the current
+ *   implementation when direct reclaim and memory compaction have failed to
+ *   allow the allocation to succeed unless __GFP_FS is also used (and some
+ *   other conditions are met), e.g. GFP_NOFS | __GFP_NOFAIL allocation has
+ *   possibility of lockup. To reduce the possibility of lockup, __GFP_HIGH is
+ *   implicitly granted by the current implementation if __GFP_NOFAIL is used.
+ *   New users of __GFP_NOFAIL should be evaluated carefully (and __GFP_NOFAIL
+ *   should be used only when there is no reasonable failure policy) but it is
+ *   definitely preferable to use __GFP_NOFAIL rather than opencode endless
+ *   loop around allocator, for a stall detection check inside allocator will
+ *   likely be able to emit possible lockup warnings unless __GFP_NOWARN is
+ *   also used.
+ *
+ * __GFP_NORETRY: The VM implementation must give up as soon as direct reclaim
+ *   and memory compaction have failed to allow the allocation to succeed.
+ *   Therefore, __GFP_NORETRY cannot be used with __GFP_NOFAIL.
  */
 #define __GFP_IO	((__force gfp_t)___GFP_IO)
 #define __GFP_FS	((__force gfp_t)___GFP_FS)

I do not like "mm, oom: get rid of TIF_MEMDIE" series because you have not
gotten confirmation from all users who might be affected (e.g. start failing
inside do_exit() which currently do not fail) by that series. If you clarify
possible side effects and get confirmation from affected users (in case some
users might need to add __GFP_NOFAIL), I will be OK with that series as well.