From 78a8d7c8ca3f0cb5cd2a276c6fc17c8c006d0b3c Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 19 Jul 2010 16:50:05 -0400 Subject: nfsd: fix error handling when starting nfsd with rpcbind down The refcounting for nfsd is a little goofy. What happens is that we create the nfsd RPC service, attach sockets to it but don't actually start the threads until someone writes to the "threads" procfile. To do this, __write_ports_addfd will create the nfsd service and then will decrement the refcount when exiting but won't actually destroy the service. This is fine when there aren't errors, but when there are this can cause later attempts to start nfsd to fail. nfsd_serv will be set, and that causes __write_versions to return EBUSY. Fix this by calling svc_destroy on nfsd_serv when this function is going to return error. Signed-off-by: Jeff Layton Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 508941c23af7..af7469efc61c 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -950,14 +950,18 @@ static ssize_t __write_ports_addfd(char *buf) return err; err = lockd_up(); - if (err != 0) - goto out; + if (err != 0) { + svc_destroy(nfsd_serv); + return err; + } err = svc_addsock(nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT); - if (err < 0) + if (err < 0) { lockd_down(); + svc_destroy(nfsd_serv); + return err; + } -out: /* Decrease the count, but don't shut down the service */ nfsd_serv->sv_nrthreads--; return err; -- cgit v1.2.2 From 0cd14a061e32d4ddaadad24d86d06cc860010591 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 19 Jul 2010 16:50:06 -0400 Subject: nfsd: fix error handling in __write_ports_addxprt __write_ports_addxprt calls nfsd_create_serv. That increases the refcount of nfsd_serv (which is tracked in sv_nrthreads). The service only decrements the thread count on error, not on success like __write_ports_addfd does, so using this interface leaves the nfsd thread count high. Fix this by having this function call svc_destroy() on error to release the reference (and possibly to tear down the service) and simply decrement the refcount without tearing down the service on success. This makes the sv_threads handling work basically the same in both __write_ports_addxprt and __write_ports_addfd. Signed-off-by: Jeff Layton Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index af7469efc61c..9e8645a07fca 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1018,6 +1018,9 @@ static ssize_t __write_ports_addxprt(char *buf) PF_INET6, port, SVC_SOCK_ANONYMOUS); if (err < 0 && err != -EAFNOSUPPORT) goto out_close; + + /* Decrease the count, but don't shut down the service */ + nfsd_serv->sv_nrthreads--; return 0; out_close: xprt = svc_find_xprt(nfsd_serv, transport, PF_INET, port); @@ -1026,8 +1029,7 @@ out_close: svc_xprt_put(xprt); } out_err: - /* Decrease the count, but don't shut down the service */ - nfsd_serv->sv_nrthreads--; + svc_destroy(nfsd_serv); return err; } -- cgit v1.2.2 From ac77efbe2b4d2a1e571a4f1e5b6e47de72a7d737 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 20 Jul 2010 14:10:22 -0400 Subject: nfsd: just keep single lockd reference for nfsd Right now, nfsd keeps a lockd reference for each socket that it has open. This is unnecessary and complicates the error handling on startup and shutdown. Change it to just do a lockd_up when starting the first nfsd thread just do a single lockd_down when taking down the last nfsd thread. Because of the strange way the sv_count is handled this requires an extra flag to tell whether the nfsd_serv holds a reference for lockd or not. Signed-off-by: Jeff Layton Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 9e8645a07fca..b1c5be85bea5 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -949,15 +949,8 @@ static ssize_t __write_ports_addfd(char *buf) if (err != 0) return err; - err = lockd_up(); - if (err != 0) { - svc_destroy(nfsd_serv); - return err; - } - err = svc_addsock(nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT); if (err < 0) { - lockd_down(); svc_destroy(nfsd_serv); return err; } @@ -982,9 +975,6 @@ static ssize_t __write_ports_delfd(char *buf) if (nfsd_serv != NULL) len = svc_sock_names(nfsd_serv, buf, SIMPLE_TRANSACTION_LIMIT, toclose); - if (len >= 0) - lockd_down(); - kfree(toclose); return len; } -- cgit v1.2.2 From 69049961014992f50b10d6c3cd3cd172d4aae5ac Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Tue, 20 Jul 2010 15:24:27 -0700 Subject: gcc-4.6: nfsd: fix initialized but not read warnings Fixes at least one real minor bug: the nfs4 recovery dir sysctl would not return its status properly. Also I finished Al's 1e41568d7378d ("Take ima_path_check() in nfsd past dentry_open() in nfsd_open()") commit, it moved the IMA code, but left the old path initializer in there. The rest is just dead code removed I think, although I was not fully sure about the "is_borc" stuff. Some more review would be still good. Found by gcc 4.6's new warnings. Signed-off-by: Andi Kleen Cc: Al Viro Cc: Neil Brown Signed-off-by: Andrew Morton Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index b1c5be85bea5..12f0ee7d1aba 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1306,6 +1306,8 @@ static ssize_t __write_recoverydir(struct file *file, char *buf, size_t size) return -EINVAL; status = nfs4_reset_recoverydir(recdir); + if (status) + return status; } return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%s\n", -- cgit v1.2.2 From 7fa53cc872332b265bc5ba1266f39586f218ad4a Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Fri, 6 Aug 2010 18:00:33 -0400 Subject: nfsd: don't allow setting maxblksize after svc created It's harmless to set this after the server is created, but also ineffective, since the value is only used at the time of svc_create_pooled(). So fail the attempt, in keeping with the pattern set by write_versions, write_{lease,grace}time and write_recoverydir. (This could break userspace that tried to write to nfsd/max_block_size between setting up sockets and starting the server. However, such code wouldn't have worked anyway, and I don't know of any examples--rpc.nfsd in nfs-utils, probably the only user of the interface, doesn't do that.) Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 12f0ee7d1aba..b53b1d042f1f 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1190,7 +1190,7 @@ static ssize_t write_maxblksize(struct file *file, char *buf, size_t size) bsize = NFSSVC_MAXBLKSIZE; bsize &= ~(1024-1); mutex_lock(&nfsd_mutex); - if (nfsd_serv && nfsd_serv->sv_nrthreads) { + if (nfsd_serv) { mutex_unlock(&nfsd_mutex); return -EBUSY; } -- cgit v1.2.2