[PATCH 6/7] cifs: Don't use PF_MEMALLOC
smfrench at gmail.com
Tue Nov 17 09:40:49 MST 2009
It is hard to follow exactly what this flag does in /mm (other than try
harder on memory allocations) - I haven't found much about this flag (e.g.
http://lwn.net/Articles/246928/) but it does look like most of the fs no
longer set this (except xfs) e.g. ext3_ordered_writepage. When running out
of memory in the cifs_demultiplex_thread it will retry 3 seconds later, but
if memory allocations ever fail in this path we could potentially be holding
up (an already issued write in) writepages for that period by not having
memory to get the response to see if the write succeeded.
We pass in few flags for these memory allocation requests: GFP_NOFS (on the
mempool_alloc) and SLAB_HWCACHE_ALIGN (on the kmem_cache_create of the pool)
should we be passing in other flags on the allocations?
On Tue, Nov 17, 2009 at 6:47 AM, Jeff Layton <jlayton at redhat.com> wrote:
> On Tue, 17 Nov 2009 16:22:32 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro at jp.fujitsu.com> wrote:
> > Non MM subsystem must not use PF_MEMALLOC. Memory reclaim need few
> > memory, anyone must not prevent it. Otherwise the system cause
> > mysterious hang-up and/or OOM Killer invokation.
> > Cc: Steve French <sfrench at samba.org>
> > Cc: linux-cifs-client at lists.samba.org
> > Cc: samba-technical at lists.samba.org
> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro at jp.fujitsu.com>
> > ---
> > fs/cifs/connect.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 63ea83f..f9b1553 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -337,7 +337,6 @@ cifs_demultiplex_thread(struct TCP_Server_Info
> > bool isMultiRsp;
> > int reconnect;
> > - current->flags |= PF_MEMALLOC;
> > cFYI(1, ("Demultiplex PID: %d", task_pid_nr(current)));
> > length = atomic_inc_return(&tcpSesAllocCount);
> This patch appears to be safe for CIFS. I believe that the demultiplex
> thread only does mempool allocations currently. The only other case
> where it did an allocation was recently changed with the conversion of
> the oplock break code to use slow_work.
> Barring anything I've missed...
> Acked-by: Jeff Layton <jlayton at redhat.com>
More information about the samba-technical