| Message ID | 1397846704-14567-12-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 18, 2014 at 02:44:05PM -0400, Trond Myklebust wrote: > Ensure that we add/remove the dl_perfile under the inode->i_lock Looks like maybe this addresses my question on the previous patch--in which case it should really be merged with the previous patch for bisectability? --b. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfsd/nfs4state.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 85fcbc9ebd40..3656980b5d19 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -441,7 +441,9 @@ static void > hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp) > { > dp->dl_stid.sc_type = NFS4_DELEG_STID; > + spin_lock(&fp->fi_inode->i_lock); > list_add(&dp->dl_perfile, &fp->fi_delegations); > + spin_unlock(&fp->fi_inode->i_lock); > list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations); > } > > @@ -449,13 +451,19 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp) > static void > unhash_delegation(struct nfs4_delegation *dp) > { > + struct nfs4_file *fp = dp->dl_file; > + > spin_lock(&state_lock); > list_del_init(&dp->dl_perclnt); > - list_del_init(&dp->dl_perfile); > list_del_init(&dp->dl_recall_lru); > + if (!list_empty(&dp->dl_perfile)) { > + spin_lock(&fp->fi_inode->i_lock); > + list_del_init(&dp->dl_perfile); > + spin_unlock(&fp->fi_inode->i_lock); > + } > spin_unlock(&state_lock); > - nfs4_put_deleg_lease(dp->dl_file); > - put_nfs4_file(dp->dl_file); > + nfs4_put_deleg_lease(fp); > + put_nfs4_file(fp); > dp->dl_file = NULL; > } > > -- > 1.9.0 > -- 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
On Tue, May 6, 2014 at 7:41 PM, Bruce Fields <bfields@fieldses.org> wrote: > On Fri, Apr 18, 2014 at 02:44:05PM -0400, Trond Myklebust wrote: >> Ensure that we add/remove the dl_perfile under the inode->i_lock > > Looks like maybe this addresses my question on the previous patch--in > which case it should really be merged with the previous patch for > bisectability? > We could, but the client_mutex will continue to mask any conflicts. In the case where it doesn't, it would be because the existing code is already borken. It really is quite non-trivial to work out all the relationships that are at work here, and to order the patches in a sane way. I'm quite open to suggestions for improvements, but when in doubt, I'm relying on the 'big umbrella' being up there until it is removed...
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 85fcbc9ebd40..3656980b5d19 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -441,7 +441,9 @@ static void hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp) { dp->dl_stid.sc_type = NFS4_DELEG_STID; + spin_lock(&fp->fi_inode->i_lock); list_add(&dp->dl_perfile, &fp->fi_delegations); + spin_unlock(&fp->fi_inode->i_lock); list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations); } @@ -449,13 +451,19 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp) static void unhash_delegation(struct nfs4_delegation *dp) { + struct nfs4_file *fp = dp->dl_file; + spin_lock(&state_lock); list_del_init(&dp->dl_perclnt); - list_del_init(&dp->dl_perfile); list_del_init(&dp->dl_recall_lru); + if (!list_empty(&dp->dl_perfile)) { + spin_lock(&fp->fi_inode->i_lock); + list_del_init(&dp->dl_perfile); + spin_unlock(&fp->fi_inode->i_lock); + } spin_unlock(&state_lock); - nfs4_put_deleg_lease(dp->dl_file); - put_nfs4_file(dp->dl_file); + nfs4_put_deleg_lease(fp); + put_nfs4_file(fp); dp->dl_file = NULL; }
Ensure that we add/remove the dl_perfile under the inode->i_lock Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfsd/nfs4state.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)