| Message ID | 943cfe47-d48a-4a55-3738-e93370160508@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd4: add refcount for nfsd4_blocked_lock | expand |
Hi Vasily- > On Dec 17, 2021, at 1:49 AM, Vasily Averin <vvs@virtuozzo.com> wrote: > > nbl allocated in nfsd4_lock can be released by a several ways: > directly in nfsd4_lock(), via nfs4_laundromat(), via another nfs > command RELEASE_LOCKOWNER or via nfsd4_callback. > This structure should be refcounted to be used and released correctly > in all these cases. > > Refcount is initialized to 1 during allocation and is incremented > when nbl is added into nbl_list/nbl_lru lists. > > Usually nbl is linked into both lists together, so only one refcount > is used for both lists. > > However nfsd4_lock() should keep in mind that nbl can be present > in one of lists only. This can happen if nbl was handled already > by nfs4_laundromat/nfsd4_callback/etc. > > Refcount is decremented if vfs_lock_file() returns FILE_LOCK_DEFERRED, > because nbl can be handled already by nfs4_laundromat/nfsd4_callback/etc. > > Refcount is not changed in find_blocked_lock() because of it reuses counter > released after removing nbl from lists. > > Signed-off-by: Vasily Averin <vvs@virtuozzo.com> > Reviewed-by: Jeff Layton <jlayton@kernel.org> Thanks. Applied provisionally to the for-next branch at git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git > --- > fs/nfsd/nfs4state.c | 25 ++++++++++++++++++++++--- > fs/nfsd/state.h | 1 + > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index d75e1235c4f5..b74f36e9901c 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -246,6 +246,7 @@ find_blocked_lock(struct nfs4_lockowner *lo, struct knfsd_fh *fh, > list_for_each_entry(cur, &lo->lo_blocked, nbl_list) { > if (fh_match(fh, &cur->nbl_fh)) { > list_del_init(&cur->nbl_list); > + WARN_ON(list_empty(&cur->nbl_lru)); > list_del_init(&cur->nbl_lru); > found = cur; > break; > @@ -271,6 +272,7 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh, > INIT_LIST_HEAD(&nbl->nbl_lru); > fh_copy_shallow(&nbl->nbl_fh, fh); > locks_init_lock(&nbl->nbl_lock); > + kref_init(&nbl->nbl_kref); > nfsd4_init_cb(&nbl->nbl_cb, lo->lo_owner.so_client, > &nfsd4_cb_notify_lock_ops, > NFSPROC4_CLNT_CB_NOTIFY_LOCK); > @@ -279,12 +281,21 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh, > return nbl; > } > > +static void > +free_nbl(struct kref *kref) > +{ > + struct nfsd4_blocked_lock *nbl; > + > + nbl = container_of(kref, struct nfsd4_blocked_lock, nbl_kref); > + kfree(nbl); > +} > + > static void > free_blocked_lock(struct nfsd4_blocked_lock *nbl) > { > locks_delete_block(&nbl->nbl_lock); > locks_release_private(&nbl->nbl_lock); > - kfree(nbl); > + kref_put(&nbl->nbl_kref, free_nbl); > } > > static void > @@ -302,6 +313,7 @@ remove_blocked_locks(struct nfs4_lockowner *lo) > struct nfsd4_blocked_lock, > nbl_list); > list_del_init(&nbl->nbl_list); > + WARN_ON(list_empty(&nbl->nbl_lru)); > list_move(&nbl->nbl_lru, &reaplist); > } > spin_unlock(&nn->blocked_locks_lock); > @@ -6976,6 +6988,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > spin_lock(&nn->blocked_locks_lock); > list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked); > list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru); > + kref_get(&nbl->nbl_kref); > spin_unlock(&nn->blocked_locks_lock); > } > > @@ -6988,6 +7001,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > nn->somebody_reclaimed = true; > break; > case FILE_LOCK_DEFERRED: > + kref_put(&nbl->nbl_kref, free_nbl); > nbl = NULL; > fallthrough; > case -EAGAIN: /* conflock holds conflicting lock */ > @@ -7008,8 +7022,13 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > /* dequeue it if we queued it before */ > if (fl_flags & FL_SLEEP) { > spin_lock(&nn->blocked_locks_lock); > - list_del_init(&nbl->nbl_list); > - list_del_init(&nbl->nbl_lru); > + if (!list_empty(&nbl->nbl_list) && > + !list_empty(&nbl->nbl_lru)) { > + list_del_init(&nbl->nbl_list); > + list_del_init(&nbl->nbl_lru); > + kref_put(&nbl->nbl_kref, free_nbl); > + } > + /* nbl can use one of lists to be linked to reaplist */ > spin_unlock(&nn->blocked_locks_lock); > } > free_blocked_lock(nbl); > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index e73bdbb1634a..ab61dc102300 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -629,6 +629,7 @@ struct nfsd4_blocked_lock { > struct file_lock nbl_lock; > struct knfsd_fh nbl_fh; > struct nfsd4_callback nbl_cb; > + struct kref nbl_kref; > }; > > struct nfsd4_compound_state; > -- > 2.25.1 > -- Chuck Lever
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index d75e1235c4f5..b74f36e9901c 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -246,6 +246,7 @@ find_blocked_lock(struct nfs4_lockowner *lo, struct knfsd_fh *fh, list_for_each_entry(cur, &lo->lo_blocked, nbl_list) { if (fh_match(fh, &cur->nbl_fh)) { list_del_init(&cur->nbl_list); + WARN_ON(list_empty(&cur->nbl_lru)); list_del_init(&cur->nbl_lru); found = cur; break; @@ -271,6 +272,7 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh, INIT_LIST_HEAD(&nbl->nbl_lru); fh_copy_shallow(&nbl->nbl_fh, fh); locks_init_lock(&nbl->nbl_lock); + kref_init(&nbl->nbl_kref); nfsd4_init_cb(&nbl->nbl_cb, lo->lo_owner.so_client, &nfsd4_cb_notify_lock_ops, NFSPROC4_CLNT_CB_NOTIFY_LOCK); @@ -279,12 +281,21 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh, return nbl; } +static void +free_nbl(struct kref *kref) +{ + struct nfsd4_blocked_lock *nbl; + + nbl = container_of(kref, struct nfsd4_blocked_lock, nbl_kref); + kfree(nbl); +} + static void free_blocked_lock(struct nfsd4_blocked_lock *nbl) { locks_delete_block(&nbl->nbl_lock); locks_release_private(&nbl->nbl_lock); - kfree(nbl); + kref_put(&nbl->nbl_kref, free_nbl); } static void @@ -302,6 +313,7 @@ remove_blocked_locks(struct nfs4_lockowner *lo) struct nfsd4_blocked_lock, nbl_list); list_del_init(&nbl->nbl_list); + WARN_ON(list_empty(&nbl->nbl_lru)); list_move(&nbl->nbl_lru, &reaplist); } spin_unlock(&nn->blocked_locks_lock); @@ -6976,6 +6988,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, spin_lock(&nn->blocked_locks_lock); list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked); list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru); + kref_get(&nbl->nbl_kref); spin_unlock(&nn->blocked_locks_lock); } @@ -6988,6 +7001,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, nn->somebody_reclaimed = true; break; case FILE_LOCK_DEFERRED: + kref_put(&nbl->nbl_kref, free_nbl); nbl = NULL; fallthrough; case -EAGAIN: /* conflock holds conflicting lock */ @@ -7008,8 +7022,13 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, /* dequeue it if we queued it before */ if (fl_flags & FL_SLEEP) { spin_lock(&nn->blocked_locks_lock); - list_del_init(&nbl->nbl_list); - list_del_init(&nbl->nbl_lru); + if (!list_empty(&nbl->nbl_list) && + !list_empty(&nbl->nbl_lru)) { + list_del_init(&nbl->nbl_list); + list_del_init(&nbl->nbl_lru); + kref_put(&nbl->nbl_kref, free_nbl); + } + /* nbl can use one of lists to be linked to reaplist */ spin_unlock(&nn->blocked_locks_lock); } free_blocked_lock(nbl); diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index e73bdbb1634a..ab61dc102300 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -629,6 +629,7 @@ struct nfsd4_blocked_lock { struct file_lock nbl_lock; struct knfsd_fh nbl_fh; struct nfsd4_callback nbl_cb; + struct kref nbl_kref; }; struct nfsd4_compound_state;