nfsd threads need to not share the init fs_struct as they need to
manipulate umask independently. So they call unshare_fs_struct() and
are the only user of that function.
In the unlikely event that unshare_fs_struct() fails, the thread will
exit calling svc_exit_thread() BEFORE svc_thread_should_stop() reports
'true'.
This is a problem because svc_exit_thread() assumes that
svc_stop_threads() is running and consequently (in the nfsd case)
nfsd_mutex is held. This ensures that the list_del_rcu() call in
svc_exit_thread() cannot race with any other manipulation of
->sp_all_threads.
While it would be possible to add some other exclusion, doing so would
introduce unnecessary complexity. unshare_fs_struct() does not fail in
practice. So the simplest solution is to make this explicit. i.e. use
__GFP_NOFAIL which is safe on such a small allocation - about 64 bytes.
Change unshare_fs_struct() to not return any error, and remove the error
handling from nfsd().
An alternate approach would be to create a variant of
kthread_create_on_node() which didn't set CLONE_FS.
Signed-off-by: NeilBrown <neilb@xxxxxxx>
---
fs/fs_struct.c | 42 ++++++++++++++++++++-------------------
fs/nfsd/nfssvc.c | 9 +++------
include/linux/fs_struct.h | 2 +-
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/fs/fs_struct.c b/fs/fs_struct.c
index 64c2d0814ed6..49fba862e408 100644
--- a/fs/fs_struct.c
+++ b/fs/fs_struct.c
@@ -109,35 +109,39 @@ void exit_fs(struct task_struct *tsk)
}
}
+static void init_fs_struct(struct fs_struct *fs, struct fs_struct *old)
+{
+ fs->users = 1;
+ fs->in_exec = 0;
+ spin_lock_init(&fs->lock);
+ seqcount_spinlock_init(&fs->seq, &fs->lock);
+ fs->umask = old->umask;
+
+ spin_lock(&old->lock);
+ fs->root = old->root;
+ path_get(&fs->root);
+ fs->pwd = old->pwd;
+ path_get(&fs->pwd);
+ spin_unlock(&old->lock);
+}
+
struct fs_struct *copy_fs_struct(struct fs_struct *old)
{
struct fs_struct *fs = kmem_cache_alloc(fs_cachep, GFP_KERNEL);
/* We don't need to lock fs - think why ;-) */
- if (fs) {
- fs->users = 1;
- fs->in_exec = 0;
- spin_lock_init(&fs->lock);
- seqcount_spinlock_init(&fs->seq, &fs->lock);
- fs->umask = old->umask;
-
- spin_lock(&old->lock);
- fs->root = old->root;
- path_get(&fs->root);
- fs->pwd = old->pwd;
- path_get(&fs->pwd);
- spin_unlock(&old->lock);
- }
+ if (fs)
+ init_fs_struct(fs, old);
return fs;
}
-int unshare_fs_struct(void)
+void unshare_fs_struct(void)
{
struct fs_struct *fs = current->fs;
- struct fs_struct *new_fs = copy_fs_struct(fs);
+ struct fs_struct *new_fs = kmem_cache_alloc(fs_cachep,
+ GFP_KERNEL| __GFP_NOFAIL);
int kill;
- if (!new_fs)
- return -ENOMEM;
+ init_fs_struct(new_fs, fs);
task_lock(current);
spin_lock(&fs->lock);
@@ -148,8 +152,6 @@ int unshare_fs_struct(void)
if (kill)
free_fs_struct(fs);
-
- return 0;
}
EXPORT_SYMBOL_GPL(unshare_fs_struct);
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 7377422a34df..f5de04a63c6f 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -873,11 +873,9 @@ nfsd(void *vrqstp)
/* At this point, the thread shares current->fs
* with the init process. We need to create files with the
- * umask as defined by the client instead of init's umask. */
- if (unshare_fs_struct() < 0) {
- printk("Unable to start nfsd thread: out of memory\n");
- goto out;
- }
+ * umask as defined by the client instead of init's umask.
+ */
+ unshare_fs_struct();
current->fs->umask = 0;
@@ -899,7 +897,6 @@ nfsd(void *vrqstp)
atomic_dec(&nfsd_th_cnt);
-out:
/* Release the thread */
svc_exit_thread(rqstp);
return 0;
diff --git a/include/linux/fs_struct.h b/include/linux/fs_struct.h
index 783b48dedb72..8282e6c7ff29 100644
--- a/include/linux/fs_struct.h
+++ b/include/linux/fs_struct.h
@@ -22,7 +22,7 @@ extern void set_fs_root(struct fs_struct *, const struct path *);
extern void set_fs_pwd(struct fs_struct *, const struct path *);
extern struct fs_struct *copy_fs_struct(struct fs_struct *);
extern void free_fs_struct(struct fs_struct *);
-extern int unshare_fs_struct(void);
+extern void unshare_fs_struct(void);
static inline void get_fs_root(struct fs_struct *fs, struct path *root)
{
--
2.44.0
