xfs
[Top] [All Lists]

Re: [PATCH 17/19] VFS: set PF_FSTRANS while namespace_sem is held.

To: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH 17/19] VFS: set PF_FSTRANS while namespace_sem is held.
From: NeilBrown <neilb@xxxxxxx>
Date: Wed, 16 Apr 2014 15:52:30 +1000
Cc: linux-mm@xxxxxxxxx, linux-nfs@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140416044618.GX18016@xxxxxxxxxxxxxxxxxx>
References: <20140416033623.10604.69237.stgit@xxxxxxxxxxxxxx> <20140416040337.10604.86740.stgit@xxxxxxxxxxxxxx> <20140416044618.GX18016@xxxxxxxxxxxxxxxxxx>
On Wed, 16 Apr 2014 05:46:18 +0100 Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:

> On Wed, Apr 16, 2014 at 02:03:37PM +1000, NeilBrown wrote:
> > namespace_sem can be taken while various i_mutex locks are held, so we
> > need to avoid reclaim from blocking on an FS (particularly loop-back
> > NFS).
> 
> I would really prefer to deal with that differently - by explicit change of
> gfp_t arguments of allocators.
> 
> The thing is, namespace_sem is held *only* over allocations, and not a lot
> of them, at that - only mnt_alloc_id(), mnt_alloc_group_id(), alloc_vfsmnt()
> and new_mountpoint().  That is all that is allowed.
> 
> Again, actual work with filesystems (setup, shutdown, remount, pathname
> resolution, etc.) is all done outside of namespace_sem; it's held only
> for manipulations of fs/{namespace,pnode}.c data structures and the only
> reason it isn't a spinlock is that we need to do some allocations.
> 
> So I'd rather slap GFP_NOFS on those few allocations...

So something like this?  I put that in to my testing instead.

Thanks,
NeilBrown

diff --git a/fs/namespace.c b/fs/namespace.c
index 83dcd5083dbb..8e103b8c8323 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -103,7 +103,7 @@ static int mnt_alloc_id(struct mount *mnt)
        int res;
 
 retry:
-       ida_pre_get(&mnt_id_ida, GFP_KERNEL);
+       ida_pre_get(&mnt_id_ida, GFP_NOFS);
        spin_lock(&mnt_id_lock);
        res = ida_get_new_above(&mnt_id_ida, mnt_id_start, &mnt->mnt_id);
        if (!res)
@@ -134,7 +134,7 @@ static int mnt_alloc_group_id(struct mount *mnt)
 {
        int res;
 
-       if (!ida_pre_get(&mnt_group_ida, GFP_KERNEL))
+       if (!ida_pre_get(&mnt_group_ida, GFP_NOFS))
                return -ENOMEM;
 
        res = ida_get_new_above(&mnt_group_ida,
@@ -193,7 +193,7 @@ unsigned int mnt_get_count(struct mount *mnt)
 
 static struct mount *alloc_vfsmnt(const char *name)
 {
-       struct mount *mnt = kmem_cache_zalloc(mnt_cache, GFP_KERNEL);
+       struct mount *mnt = kmem_cache_zalloc(mnt_cache, GFP_NOFS);
        if (mnt) {
                int err;
 
@@ -202,7 +202,7 @@ static struct mount *alloc_vfsmnt(const char *name)
                        goto out_free_cache;
 
                if (name) {
-                       mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
+                       mnt->mnt_devname = kstrdup(name, GFP_NOFS);
                        if (!mnt->mnt_devname)
                                goto out_free_id;
                }
@@ -682,7 +682,7 @@ static struct mountpoint *new_mountpoint(struct dentry 
*dentry)
                }
        }
 
-       mp = kmalloc(sizeof(struct mountpoint), GFP_KERNEL);
+       mp = kmalloc(sizeof(struct mountpoint), GFP_NOFS);
        if (!mp)
                return ERR_PTR(-ENOMEM);
 

Attachment: signature.asc
Description: PGP signature

<Prev in Thread] Current Thread [Next in Thread>