Hi Bruce,
As I mentioned on IRC yesterday, here are a couple of patches to the
fi_delegees handling. The first one fixes a bug. I think it's pretty
unlikely to be hit in most cases, so I'm not sure if it's suitable for
stable. Your call there.
The second is just a cleanup patch -- fi_delegees is always handled
under spinlock so it really doesn't need to be an atomic_t.
Jeff Layton (2):
nfsd: fix fi_delegees leak when fi_had_conflict returns true
nfsd: fi_delegees doesn't need to be an atomic_t
fs/nfsd/nfs4state.c | 8 ++++----
fs/nfsd/state.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
--
2.1.0
Currently, nfs4_set_delegation takes a reference to an existing
delegation and then checks to see if there is a conflict. If there is
one, then it doesn't release that reference.
Change the code to take the reference after the check and only if there
is no conflict.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5f62e8051e2b..86bc2882bffc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3898,11 +3898,11 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
status = nfs4_setlease(dp);
goto out;
}
- atomic_inc(&fp->fi_delegees);
if (fp->fi_had_conflict) {
status = -EAGAIN;
goto out_unlock;
}
+ atomic_inc(&fp->fi_delegees);
hash_delegation_locked(dp, fp);
status = 0;
out_unlock:
--
2.1.0
fi_delegees is always handled under the fi_lock, so there's no need to
use an atomic_t for this field.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 8 ++++----
fs/nfsd/state.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 86bc2882bffc..55e8afeff8db 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -688,7 +688,7 @@ static void nfs4_put_deleg_lease(struct nfs4_file *fp)
struct file *filp = NULL;
spin_lock(&fp->fi_lock);
- if (fp->fi_deleg_file && atomic_dec_and_test(&fp->fi_delegees))
+ if (fp->fi_deleg_file && --fp->fi_delegees == 0)
swap(filp, fp->fi_deleg_file);
spin_unlock(&fp->fi_lock);
@@ -3856,12 +3856,12 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
/* Race breaker */
if (fp->fi_deleg_file) {
status = 0;
- atomic_inc(&fp->fi_delegees);
+ ++fp->fi_delegees;
hash_delegation_locked(dp, fp);
goto out_unlock;
}
fp->fi_deleg_file = filp;
- atomic_set(&fp->fi_delegees, 1);
+ fp->fi_delegees = 1;
hash_delegation_locked(dp, fp);
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
@@ -3902,7 +3902,7 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
status = -EAGAIN;
goto out_unlock;
}
- atomic_inc(&fp->fi_delegees);
+ ++fp->fi_delegees;
hash_delegation_locked(dp, fp);
status = 0;
out_unlock:
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 28264a72fa60..afadb7c2e357 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -489,7 +489,7 @@ struct nfs4_file {
atomic_t fi_access[2];
u32 fi_share_deny;
struct file *fi_deleg_file;
- atomic_t fi_delegees;
+ int fi_delegees;
struct knfsd_fh fi_fhandle;
bool fi_had_conflict;
};
--
2.1.0
On Sat, Dec 13, 2014 at 09:11:38AM -0500, Jeff Layton wrote:
> As I mentioned on IRC yesterday, here are a couple of patches to the
> fi_delegees handling. The first one fixes a bug. I think it's pretty
> unlikely to be hit in most cases, so I'm not sure if it's suitable for
> stable. Your call there.
>
> The second is just a cleanup patch -- fi_delegees is always handled
> under spinlock so it really doesn't need to be an atomic_t.
Sorry for the slow response, applying for 3.19 and stable (well, the
first anyway), may not get that passed along till -rc2.
--b.
>
> Jeff Layton (2):
> nfsd: fix fi_delegees leak when fi_had_conflict returns true
> nfsd: fi_delegees doesn't need to be an atomic_t
>
> fs/nfsd/nfs4state.c | 8 ++++----
> fs/nfsd/state.h | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> --
> 2.1.0
>