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 <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
---
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
Hi Vasily-
> On Dec 17, 2021, at 1:49 AM, Vasily Averin <[email protected]> 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 <[email protected]>
> Reviewed-by: Jeff Layton <[email protected]>
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