Re: [PATCH 5/6] XFS: remove congestion_wait() loop from kmem_alloc()
From: Dave Chinner <david@fromorbit.com>
Date: 2021-09-14 01:31:26
Also in:
linux-ext4, linux-fsdevel, linux-mm, linux-nfs, lkml
On Tue, Sep 14, 2021 at 10:13:04AM +1000, NeilBrown wrote:
quoted hunk ↗ jump to hunk
Documentation commment in gfp.h discourages indefinite retry loops on ENOMEM and says of __GFP_NOFAIL that it is definitely preferable to use the flag rather than opencode endless loop around allocator. So remove the loop, instead specifying __GFP_NOFAIL if KM_MAYFAIL was not given. Signed-off-by: NeilBrown <redacted> --- fs/xfs/kmem.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c index 6f49bf39183c..f545f3633f88 100644 --- a/fs/xfs/kmem.c +++ b/fs/xfs/kmem.c@@ -13,19 +13,11 @@ kmem_alloc(size_t size, xfs_km_flags_t flags) { int retries = 0; gfp_t lflags = kmem_flags_convert(flags); - void *ptr; trace_kmem_alloc(size, flags, _RET_IP_); - do { - ptr = kmalloc(size, lflags); - if (ptr || (flags & KM_MAYFAIL)) - return ptr; - if (!(++retries % 100)) - xfs_err(NULL, - "%s(%u) possible memory allocation deadlock size %u in %s (mode:0x%x)", - current->comm, current->pid, - (unsigned int)size, __func__, lflags); - congestion_wait(BLK_RW_ASYNC, HZ/50); - } while (1); + if (!(flags & KM_MAYFAIL)) + lflags |= __GFP_NOFAIL; + + return kmalloc(size, lflags); }
Which means we no longer get warnings about memory allocation failing - kmem_flags_convert() sets __GFP_NOWARN for all allocations in this loop. Hence we'll now get silent deadlocks through this code instead of getting warnings that memory allocation is failing repeatedly. I also wonder about changing the backoff behaviour here (it's a 20ms wait right now because there are not early wakeups) will affect the behaviour, as __GFP_NOFAIL won't wait for that extra time between allocation attempts.... And, of course, how did you test this? Sometimes we see unpredicted behaviours as a result of "simple" changes like this under low memory conditions... Cheers, Dave. -- Dave Chinner david@fromorbit.com