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

NeilBrown
2014-04-16T05:52:38Z

[-- Attachment #1: Type: text/plain, Size: 2562 bytes --]

On Wed, 16 Apr 2014 05:46:18 +0100 Al Viro <viro@ZenIV.linux.org.uk> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]