| Message ID | 163184741781.29351.4475236694432020436.stgit@noble.brown (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | congestion_wait() and GFP_NOFAIL | expand |
On Fri, Sep 17, 2021 at 12:56:57PM +1000, NeilBrown wrote: > 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. > > As we no longer have the opportunity to report a warning after some > failures, clear __GFP_NOWARN so that the default warning (rate-limited > to 1 ever 10 seconds) will be reported instead. > > Signed-off-by: NeilBrown <neilb@suse.de> > --- > fs/xfs/kmem.c | 19 ++++++------------- > 1 file changed, 6 insertions(+), 13 deletions(-) > > diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c > index 6f49bf39183c..575a58e61391 100644 > --- a/fs/xfs/kmem.c > +++ b/fs/xfs/kmem.c > @@ -11,21 +11,14 @@ > void * > 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; > + lflags &= ~__GFP_NOWARN; > + } This logic should really be in kmem_flags_convert() where the gfp flags are set up. kmem_flags_convert() is only called by kmem_alloc() now so you should just be able to hack that logic to do exactly what is necessary. FWIW, We've kinda not been caring about warts in this code because the next step for kmem_alloc is to remove kmem_alloc/kmem_zalloc completely and replace all the callers with kmalloc/kzalloc being passed the correct gfp flags. There's about 30 kmem_alloc() callers and 45 kmem_zalloc() calls left to be converted before kmem.[ch] can go away completely.... Cheers, Dave.
diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c index 6f49bf39183c..575a58e61391 100644 --- a/fs/xfs/kmem.c +++ b/fs/xfs/kmem.c @@ -11,21 +11,14 @@ void * 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; + lflags &= ~__GFP_NOWARN; + } + + return kmalloc(size, lflags); }
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. As we no longer have the opportunity to report a warning after some failures, clear __GFP_NOWARN so that the default warning (rate-limited to 1 ever 10 seconds) will be reported instead. Signed-off-by: NeilBrown <neilb@suse.de> --- fs/xfs/kmem.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)