* [PATCH 1/4] nfsd4: fix spurious 4.1 post-reboot failures
2012-01-03 22:56 3.3 nfsd fixes J. Bruce Fields
@ 2012-01-03 22:56 ` J. Bruce Fields
2012-01-03 22:56 ` [PATCH 2/4] nfsd4: be forgiving in the absence of the recovery directory J. Bruce Fields
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-01-03 22:56 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
In the NFSv4.1 case, this could cause a spurious "NFSD: failed to write
recovery record (err -17); please check that /var/lib/nfs/v4recovery
exists and is writable.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Reported-by: Steve Dickson <SteveD@redhat.com>
---
fs/nfsd/nfs4recover.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index ed083b9..28712e2 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -144,8 +144,15 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
status = PTR_ERR(dentry);
goto out_unlock;
}
- status = -EEXIST;
if (dentry->d_inode)
+ /*
+ * In the 4.1 case, where we're called from
+ * reclaim_complete(), records from the previous reboot
+ * may still be left, so this is OK.
+ *
+ * In the 4.0 case, we should never get here; but we may
+ * as well be forgiving and just succeed silently.
+ */
goto out_put;
status = mnt_want_write(rec_file->f_path.mnt);
if (status)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] nfsd4: be forgiving in the absence of the recovery directory
2012-01-03 22:56 3.3 nfsd fixes J. Bruce Fields
2012-01-03 22:56 ` [PATCH 1/4] nfsd4: fix spurious 4.1 post-reboot failures J. Bruce Fields
@ 2012-01-03 22:56 ` J. Bruce Fields
2012-01-03 22:56 ` [PATCH 3/4] svcrpc: fix double-free on shutdown of nfsd after changing pool mode J. Bruce Fields
2012-01-03 22:56 ` [PATCH 4/4] svcrpc: don't revert to SVC_POOL_DEFAULT on nfsd shutdown J. Bruce Fields
3 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-01-03 22:56 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
If the recovery directory doesn't exist, then behavior after a reboot
will be suboptimal. But it's unnecessarily harsh to then prevent the
nfsv4 server from working at all. Instead just print a warning
(already done in nfsd4_init_recdir()) and soldier on.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4recover.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 28712e2..eb01fff 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -127,10 +127,11 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
dprintk("NFSD: nfsd4_create_clid_dir for \"%s\"\n", dname);
- if (!rec_file || clp->cl_firststate)
+ if (clp->cl_firststate)
return 0;
-
clp->cl_firststate = 1;
+ if (!rec_file)
+ return -ENOENT;
status = nfs4_save_creds(&original_cred);
if (status < 0)
return status;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] svcrpc: fix double-free on shutdown of nfsd after changing pool mode
2012-01-03 22:56 3.3 nfsd fixes J. Bruce Fields
2012-01-03 22:56 ` [PATCH 1/4] nfsd4: fix spurious 4.1 post-reboot failures J. Bruce Fields
2012-01-03 22:56 ` [PATCH 2/4] nfsd4: be forgiving in the absence of the recovery directory J. Bruce Fields
@ 2012-01-03 22:56 ` J. Bruce Fields
2012-01-03 22:57 ` J. Bruce Fields
2012-01-03 22:56 ` [PATCH 4/4] svcrpc: don't revert to SVC_POOL_DEFAULT on nfsd shutdown J. Bruce Fields
3 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2012-01-03 22:56 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
The pool_to and to_pool fields of the global svc_pool_map are freed on
shutdown, but are initialized in nfsd startup only in the
SVC_POOL_PERCPU and SVC_POOL_PERNODE cases.
They *are* initialized to zero on kernel startup. So as long as you use
only SVC_POOL_GLOBAL (the default), this will never be a problem.
You're also OK if you only ever use SVC_POOL_PERCPU or SVC_POOL_PERNODE.
However, the following sequence events leads to a double-free:
1. set SVC_POOL_PERCPU or SVC_POOL_PERNODE
2. start nfsd: both fields are initialized.
3. shutdown nfsd: both fields are freed.
4. set SVC_POOL_GLOBAL
5. start nfsd: the fields are left untouched.
6. shutdown nfsd: now we try to free them again.
Step 4 is actually unnecessary, since (for some bizarre reason), nfsd
automatically resets the pool mode to SVC_POOL_GLOBAL on shutdown.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/svc.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index e9632bb..1dd5fd0 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -167,6 +167,7 @@ svc_pool_map_alloc_arrays(struct svc_pool_map *m, unsigned int maxpools)
fail_free:
kfree(m->to_pool);
+ m->to_pool = NULL;
fail:
return -ENOMEM;
}
@@ -287,7 +288,9 @@ svc_pool_map_put(void)
if (!--m->count) {
m->mode = SVC_POOL_DEFAULT;
kfree(m->to_pool);
+ m->to_pool = NULL;
kfree(m->pool_to);
+ m->pool_to = NULL;
m->npools = 0;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/4] svcrpc: fix double-free on shutdown of nfsd after changing pool mode
2012-01-03 22:56 ` [PATCH 3/4] svcrpc: fix double-free on shutdown of nfsd after changing pool mode J. Bruce Fields
@ 2012-01-03 22:57 ` J. Bruce Fields
0 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-01-03 22:57 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
On Tue, Jan 03, 2012 at 05:56:20PM -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> The pool_to and to_pool fields of the global svc_pool_map are freed on
> shutdown, but are initialized in nfsd startup only in the
> SVC_POOL_PERCPU and SVC_POOL_PERNODE cases.
>
> They *are* initialized to zero on kernel startup. So as long as you use
> only SVC_POOL_GLOBAL (the default), this will never be a problem.
>
> You're also OK if you only ever use SVC_POOL_PERCPU or SVC_POOL_PERNODE.
>
> However, the following sequence events leads to a double-free:
>
> 1. set SVC_POOL_PERCPU or SVC_POOL_PERNODE
> 2. start nfsd: both fields are initialized.
> 3. shutdown nfsd: both fields are freed.
> 4. set SVC_POOL_GLOBAL
> 5. start nfsd: the fields are left untouched.
> 6. shutdown nfsd: now we try to free them again.
>
> Step 4 is actually unnecessary, since (for some bizarre reason), nfsd
> automatically resets the pool mode to SVC_POOL_GLOBAL on shutdown.
>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Oops, also adding a stable cc for this.
--b.
> ---
> net/sunrpc/svc.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index e9632bb..1dd5fd0 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -167,6 +167,7 @@ svc_pool_map_alloc_arrays(struct svc_pool_map *m, unsigned int maxpools)
>
> fail_free:
> kfree(m->to_pool);
> + m->to_pool = NULL;
> fail:
> return -ENOMEM;
> }
> @@ -287,7 +288,9 @@ svc_pool_map_put(void)
> if (!--m->count) {
> m->mode = SVC_POOL_DEFAULT;
> kfree(m->to_pool);
> + m->to_pool = NULL;
> kfree(m->pool_to);
> + m->pool_to = NULL;
> m->npools = 0;
> }
>
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/4] svcrpc: don't revert to SVC_POOL_DEFAULT on nfsd shutdown
2012-01-03 22:56 3.3 nfsd fixes J. Bruce Fields
` (2 preceding siblings ...)
2012-01-03 22:56 ` [PATCH 3/4] svcrpc: fix double-free on shutdown of nfsd after changing pool mode J. Bruce Fields
@ 2012-01-03 22:56 ` J. Bruce Fields
3 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2012-01-03 22:56 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
This was unexpected behavior (at least for me)--why would you want
configuration settings automatically lost on nfsd restart?
In practice this won't affect distributions, which likely set everything
on every startup. But I'd expect the behavior to be less confusing to
someone manually restarting nfsd for testing.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/svc.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 1dd5fd0..9701798 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -286,7 +286,6 @@ svc_pool_map_put(void)
mutex_lock(&svc_pool_map_mutex);
if (!--m->count) {
- m->mode = SVC_POOL_DEFAULT;
kfree(m->to_pool);
m->to_pool = NULL;
kfree(m->pool_to);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread