nfsd4_release_lockowner() mustn't hold clp->cl_lock when
check_for_locks() invokes nfsd_file_put(), which can sleep.
Reported-by: Dai Ngo <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
Cc: <[email protected]>
---
fs/nfsd/nfs4state.c | 56 +++++++++++++++++++++++----------------------------
1 file changed, 25 insertions(+), 31 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 234e852fcdfa..e2eb6d031643 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6611,8 +6611,8 @@ nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
deny->ld_type = NFS4_WRITE_LT;
}
-static struct nfs4_lockowner *
-find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
+static struct nfs4_stateowner *
+find_stateowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
{
unsigned int strhashval = ownerstr_hashval(owner);
struct nfs4_stateowner *so;
@@ -6624,11 +6624,22 @@ find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
if (so->so_is_open_owner)
continue;
if (same_owner_str(so, owner))
- return lockowner(nfs4_get_stateowner(so));
+ return nfs4_get_stateowner(so);
}
return NULL;
}
+static struct nfs4_lockowner *
+find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
+{
+ struct nfs4_stateowner *so;
+
+ so = find_stateowner_str_locked(clp, owner);
+ if (!so)
+ return NULL;
+ return lockowner(so);
+}
+
static struct nfs4_lockowner *
find_lockowner_str(struct nfs4_client *clp, struct xdr_netobj *owner)
{
@@ -7305,10 +7316,8 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
struct nfsd4_release_lockowner *rlockowner = &u->release_lockowner;
clientid_t *clid = &rlockowner->rl_clientid;
struct nfs4_stateowner *sop;
- struct nfs4_lockowner *lo = NULL;
+ struct nfs4_lockowner *lo;
struct nfs4_ol_stateid *stp;
- struct xdr_netobj *owner = &rlockowner->rl_owner;
- unsigned int hashval = ownerstr_hashval(owner);
__be32 status;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
struct nfs4_client *clp;
@@ -7322,32 +7331,18 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
return status;
clp = cstate->clp;
- /* Find the matching lock stateowner */
spin_lock(&clp->cl_lock);
- list_for_each_entry(sop, &clp->cl_ownerstr_hashtbl[hashval],
- so_strhash) {
-
- if (sop->so_is_open_owner || !same_owner_str(sop, owner))
- continue;
-
- /* see if there are still any locks associated with it */
- lo = lockowner(sop);
- list_for_each_entry(stp, &sop->so_stateids, st_perstateowner) {
- if (check_for_locks(stp->st_stid.sc_file, lo)) {
- status = nfserr_locks_held;
- spin_unlock(&clp->cl_lock);
- return status;
- }
- }
+ sop = find_stateowner_str_locked(clp, &rlockowner->rl_owner);
+ spin_unlock(&clp->cl_lock);
+ if (!sop)
+ return nfs_ok;
- nfs4_get_stateowner(sop);
- break;
- }
- if (!lo) {
- spin_unlock(&clp->cl_lock);
- return status;
- }
+ lo = lockowner(sop);
+ list_for_each_entry(stp, &sop->so_stateids, st_perstateowner)
+ if (check_for_locks(stp->st_stid.sc_file, lo))
+ return nfserr_locks_held;
+ spin_lock(&clp->cl_lock);
unhash_lockowner_locked(lo);
while (!list_empty(&lo->lo_owner.so_stateids)) {
stp = list_first_entry(&lo->lo_owner.so_stateids,
@@ -7360,8 +7355,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
free_ol_stateid_reaplist(&reaplist);
remove_blocked_locks(lo);
nfs4_put_stateowner(&lo->lo_owner);
-
- return status;
+ return nfs_ok;
}
static inline struct nfs4_client_reclaim *
> On May 11, 2022, at 6:04 PM, Trond Myklebust <[email protected]> wrote:
>
> On Wed, 2022-05-11 at 17:52 -0400, Chuck Lever wrote:
>> nfsd4_release_lockowner() mustn't hold clp->cl_lock when
>> check_for_locks() invokes nfsd_file_put(), which can sleep.
>>
>> Reported-by: Dai Ngo <[email protected]>
>> Signed-off-by: Chuck Lever <[email protected]>
>> Cc: <[email protected]>
>> ---
>> fs/nfsd/nfs4state.c | 56 +++++++++++++++++++++++------------------
>> ----------
>> 1 file changed, 25 insertions(+), 31 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 234e852fcdfa..e2eb6d031643 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -6611,8 +6611,8 @@ nfs4_set_lock_denied(struct file_lock *fl,
>> struct nfsd4_lock_denied *deny)
>> deny->ld_type = NFS4_WRITE_LT;
>> }
>>
>> -static struct nfs4_lockowner *
>> -find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj
>> *owner)
>> +static struct nfs4_stateowner *
>> +find_stateowner_str_locked(struct nfs4_client *clp, struct
>> xdr_netobj *owner)
>> {
>> unsigned int strhashval = ownerstr_hashval(owner);
>> struct nfs4_stateowner *so;
>> @@ -6624,11 +6624,22 @@ find_lockowner_str_locked(struct nfs4_client
>> *clp, struct xdr_netobj *owner)
>> if (so->so_is_open_owner)
>> continue;
>> if (same_owner_str(so, owner))
>> - return lockowner(nfs4_get_stateowner(so));
>> + return nfs4_get_stateowner(so);
>
> Hmm... If nfs4_get_stateowner() can fail here, don't you want to
> continue searching the list when it does?
You've lost me on this.
494 static inline struct nfs4_stateowner *
495 nfs4_get_stateowner(struct nfs4_stateowner *sop)
496 {
497 atomic_inc(&sop->so_count);
498 return sop;
499 }
500
How would nfs4_get_stateowner() fail?
>> }
>> return NULL;
>> }
>>
>> +static struct nfs4_lockowner *
>> +find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj
>> *owner)
>> +{
>> + struct nfs4_stateowner *so;
>> +
>> + so = find_stateowner_str_locked(clp, owner);
>> + if (!so)
>> + return NULL;
>> + return lockowner(so);
>> +}
>> +
>> static struct nfs4_lockowner *
>> find_lockowner_str(struct nfs4_client *clp, struct xdr_netobj
>> *owner)
>> {
>> @@ -7305,10 +7316,8 @@ nfsd4_release_lockowner(struct svc_rqst
>> *rqstp,
>> struct nfsd4_release_lockowner *rlockowner = &u-
>>> release_lockowner;
>> clientid_t *clid = &rlockowner->rl_clientid;
>> struct nfs4_stateowner *sop;
>> - struct nfs4_lockowner *lo = NULL;
>> + struct nfs4_lockowner *lo;
>> struct nfs4_ol_stateid *stp;
>> - struct xdr_netobj *owner = &rlockowner->rl_owner;
>> - unsigned int hashval = ownerstr_hashval(owner);
>> __be32 status;
>> struct nfsd_net *nn = net_generic(SVC_NET(rqstp),
>> nfsd_net_id);
>> struct nfs4_client *clp;
>> @@ -7322,32 +7331,18 @@ nfsd4_release_lockowner(struct svc_rqst
>> *rqstp,
>> return status;
>>
>> clp = cstate->clp;
>> - /* Find the matching lock stateowner */
>> spin_lock(&clp->cl_lock);
>> - list_for_each_entry(sop, &clp->cl_ownerstr_hashtbl[hashval],
>> - so_strhash) {
>> -
>> - if (sop->so_is_open_owner || !same_owner_str(sop,
>> owner))
>> - continue;
>> -
>> - /* see if there are still any locks associated with
>> it */
>> - lo = lockowner(sop);
>> - list_for_each_entry(stp, &sop->so_stateids,
>> st_perstateowner) {
>> - if (check_for_locks(stp->st_stid.sc_file,
>> lo)) {
>> - status = nfserr_locks_held;
>> - spin_unlock(&clp->cl_lock);
>> - return status;
>> - }
>> - }
>> + sop = find_stateowner_str_locked(clp, &rlockowner->rl_owner);
>> + spin_unlock(&clp->cl_lock);
>> + if (!sop)
>> + return nfs_ok;
>>
>> - nfs4_get_stateowner(sop);
>> - break;
>> - }
>> - if (!lo) {
>> - spin_unlock(&clp->cl_lock);
>> - return status;
>> - }
>> + lo = lockowner(sop);
>> + list_for_each_entry(stp, &sop->so_stateids, st_perstateowner)
>> + if (check_for_locks(stp->st_stid.sc_file, lo))
>> + return nfserr_locks_held;
>
> Isn't this line now leaking the reference to sop?
Indeed. Fixed.
>>
>> + spin_lock(&clp->cl_lock);
>> unhash_lockowner_locked(lo);
>> while (!list_empty(&lo->lo_owner.so_stateids)) {
>> stp = list_first_entry(&lo->lo_owner.so_stateids,
>> @@ -7360,8 +7355,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
>> free_ol_stateid_reaplist(&reaplist);
>> remove_blocked_locks(lo);
>> nfs4_put_stateowner(&lo->lo_owner);
>> -
>> - return status;
>> + return nfs_ok;
>> }
>>
>> static inline struct nfs4_client_reclaim *
>>
>>
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> [email protected]
>
>
--
Chuck Lever
On Wed, 2022-05-11 at 17:52 -0400, Chuck Lever wrote:
> nfsd4_release_lockowner() mustn't hold clp->cl_lock when
> check_for_locks() invokes nfsd_file_put(), which can sleep.
>
> Reported-by: Dai Ngo <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
> Cc: <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 56 +++++++++++++++++++++++------------------
> ----------
> 1 file changed, 25 insertions(+), 31 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 234e852fcdfa..e2eb6d031643 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6611,8 +6611,8 @@ nfs4_set_lock_denied(struct file_lock *fl,
> struct nfsd4_lock_denied *deny)
> deny->ld_type = NFS4_WRITE_LT;
> }
>
> -static struct nfs4_lockowner *
> -find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj
> *owner)
> +static struct nfs4_stateowner *
> +find_stateowner_str_locked(struct nfs4_client *clp, struct
> xdr_netobj *owner)
> {
> unsigned int strhashval = ownerstr_hashval(owner);
> struct nfs4_stateowner *so;
> @@ -6624,11 +6624,22 @@ find_lockowner_str_locked(struct nfs4_client
> *clp, struct xdr_netobj *owner)
> if (so->so_is_open_owner)
> continue;
> if (same_owner_str(so, owner))
> - return lockowner(nfs4_get_stateowner(so));
> + return nfs4_get_stateowner(so);
Hmm... If nfs4_get_stateowner() can fail here, don't you want to
continue searching the list when it does?
> }
> return NULL;
> }
>
> +static struct nfs4_lockowner *
> +find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj
> *owner)
> +{
> + struct nfs4_stateowner *so;
> +
> + so = find_stateowner_str_locked(clp, owner);
> + if (!so)
> + return NULL;
> + return lockowner(so);
> +}
> +
> static struct nfs4_lockowner *
> find_lockowner_str(struct nfs4_client *clp, struct xdr_netobj
> *owner)
> {
> @@ -7305,10 +7316,8 @@ nfsd4_release_lockowner(struct svc_rqst
> *rqstp,
> struct nfsd4_release_lockowner *rlockowner = &u-
> >release_lockowner;
> clientid_t *clid = &rlockowner->rl_clientid;
> struct nfs4_stateowner *sop;
> - struct nfs4_lockowner *lo = NULL;
> + struct nfs4_lockowner *lo;
> struct nfs4_ol_stateid *stp;
> - struct xdr_netobj *owner = &rlockowner->rl_owner;
> - unsigned int hashval = ownerstr_hashval(owner);
> __be32 status;
> struct nfsd_net *nn = net_generic(SVC_NET(rqstp),
> nfsd_net_id);
> struct nfs4_client *clp;
> @@ -7322,32 +7331,18 @@ nfsd4_release_lockowner(struct svc_rqst
> *rqstp,
> return status;
>
> clp = cstate->clp;
> - /* Find the matching lock stateowner */
> spin_lock(&clp->cl_lock);
> - list_for_each_entry(sop, &clp->cl_ownerstr_hashtbl[hashval],
> - so_strhash) {
> -
> - if (sop->so_is_open_owner || !same_owner_str(sop,
> owner))
> - continue;
> -
> - /* see if there are still any locks associated with
> it */
> - lo = lockowner(sop);
> - list_for_each_entry(stp, &sop->so_stateids,
> st_perstateowner) {
> - if (check_for_locks(stp->st_stid.sc_file,
> lo)) {
> - status = nfserr_locks_held;
> - spin_unlock(&clp->cl_lock);
> - return status;
> - }
> - }
> + sop = find_stateowner_str_locked(clp, &rlockowner->rl_owner);
> + spin_unlock(&clp->cl_lock);
> + if (!sop)
> + return nfs_ok;
>
> - nfs4_get_stateowner(sop);
> - break;
> - }
> - if (!lo) {
> - spin_unlock(&clp->cl_lock);
> - return status;
> - }
> + lo = lockowner(sop);
> + list_for_each_entry(stp, &sop->so_stateids, st_perstateowner)
> + if (check_for_locks(stp->st_stid.sc_file, lo))
> + return nfserr_locks_held;
Isn't this line now leaking the reference to sop?
>
> + spin_lock(&clp->cl_lock);
> unhash_lockowner_locked(lo);
> while (!list_empty(&lo->lo_owner.so_stateids)) {
> stp = list_first_entry(&lo->lo_owner.so_stateids,
> @@ -7360,8 +7355,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> free_ol_stateid_reaplist(&reaplist);
> remove_blocked_locks(lo);
> nfs4_put_stateowner(&lo->lo_owner);
> -
> - return status;
> + return nfs_ok;
> }
>
> static inline struct nfs4_client_reclaim *
>
>
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Wed, 2022-05-11 at 17:52 -0400, Chuck Lever wrote:
> nfsd4_release_lockowner() mustn't hold clp->cl_lock when
> check_for_locks() invokes nfsd_file_put(), which can sleep.
>
> Reported-by: Dai Ngo <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
> Cc: <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 56 +++++++++++++++++++++++----------------------------
> 1 file changed, 25 insertions(+), 31 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 234e852fcdfa..e2eb6d031643 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6611,8 +6611,8 @@ nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
> deny->ld_type = NFS4_WRITE_LT;
> }
>
> -static struct nfs4_lockowner *
> -find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
> +static struct nfs4_stateowner *
> +find_stateowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
> {
> unsigned int strhashval = ownerstr_hashval(owner);
> struct nfs4_stateowner *so;
> @@ -6624,11 +6624,22 @@ find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
> if (so->so_is_open_owner)
> continue;
> if (same_owner_str(so, owner))
> - return lockowner(nfs4_get_stateowner(so));
> + return nfs4_get_stateowner(so);
> }
> return NULL;
> }
>
> +static struct nfs4_lockowner *
> +find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
> +{
> + struct nfs4_stateowner *so;
> +
> + so = find_stateowner_str_locked(clp, owner);
> + if (!so)
> + return NULL;
> + return lockowner(so);
> +}
> +
> static struct nfs4_lockowner *
> find_lockowner_str(struct nfs4_client *clp, struct xdr_netobj *owner)
> {
> @@ -7305,10 +7316,8 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> struct nfsd4_release_lockowner *rlockowner = &u->release_lockowner;
> clientid_t *clid = &rlockowner->rl_clientid;
> struct nfs4_stateowner *sop;
> - struct nfs4_lockowner *lo = NULL;
> + struct nfs4_lockowner *lo;
> struct nfs4_ol_stateid *stp;
> - struct xdr_netobj *owner = &rlockowner->rl_owner;
> - unsigned int hashval = ownerstr_hashval(owner);
> __be32 status;
> struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> struct nfs4_client *clp;
> @@ -7322,32 +7331,18 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> return status;
>
> clp = cstate->clp;
> - /* Find the matching lock stateowner */
> spin_lock(&clp->cl_lock);
> - list_for_each_entry(sop, &clp->cl_ownerstr_hashtbl[hashval],
> - so_strhash) {
> -
> - if (sop->so_is_open_owner || !same_owner_str(sop, owner))
> - continue;
> -
> - /* see if there are still any locks associated with it */
> - lo = lockowner(sop);
> - list_for_each_entry(stp, &sop->so_stateids, st_perstateowner) {
> - if (check_for_locks(stp->st_stid.sc_file, lo)) {
> - status = nfserr_locks_held;
> - spin_unlock(&clp->cl_lock);
> - return status;
> - }
> - }
> + sop = find_stateowner_str_locked(clp, &rlockowner->rl_owner);
> + spin_unlock(&clp->cl_lock);
> + if (!sop)
> + return nfs_ok;
>
> - nfs4_get_stateowner(sop);
> - break;
> - }
> - if (!lo) {
> - spin_unlock(&clp->cl_lock);
> - return status;
> - }
> + lo = lockowner(sop);
> + list_for_each_entry(stp, &sop->so_stateids, st_perstateowner)
> + if (check_for_locks(stp->st_stid.sc_file, lo))
> + return nfserr_locks_held;
>
It has been a while since I was in this code, but is it safe to walk the
above list without holding the cl_lock?
> + spin_lock(&clp->cl_lock);
> unhash_lockowner_locked(lo);
> while (!list_empty(&lo->lo_owner.so_stateids)) {
> stp = list_first_entry(&lo->lo_owner.so_stateids,
> @@ -7360,8 +7355,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> free_ol_stateid_reaplist(&reaplist);
> remove_blocked_locks(lo);
> nfs4_put_stateowner(&lo->lo_owner);
> -
> - return status;
> + return nfs_ok;
> }
>
> static inline struct nfs4_client_reclaim *
>
>
--
Jeff Layton <[email protected]>
> On May 12, 2022, at 6:07 AM, Jeff Layton <[email protected]> wrote:
>
> On Wed, 2022-05-11 at 17:52 -0400, Chuck Lever wrote:
>> nfsd4_release_lockowner() mustn't hold clp->cl_lock when
>> check_for_locks() invokes nfsd_file_put(), which can sleep.
>>
>> Reported-by: Dai Ngo <[email protected]>
>> Signed-off-by: Chuck Lever <[email protected]>
>> Cc: <[email protected]>
>> ---
>> fs/nfsd/nfs4state.c | 56 +++++++++++++++++++++++----------------------------
>> 1 file changed, 25 insertions(+), 31 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 234e852fcdfa..e2eb6d031643 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -6611,8 +6611,8 @@ nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
>> deny->ld_type = NFS4_WRITE_LT;
>> }
>>
>> -static struct nfs4_lockowner *
>> -find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
>> +static struct nfs4_stateowner *
>> +find_stateowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
>> {
>> unsigned int strhashval = ownerstr_hashval(owner);
>> struct nfs4_stateowner *so;
>> @@ -6624,11 +6624,22 @@ find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
>> if (so->so_is_open_owner)
>> continue;
>> if (same_owner_str(so, owner))
>> - return lockowner(nfs4_get_stateowner(so));
>> + return nfs4_get_stateowner(so);
>> }
>> return NULL;
>> }
>>
>> +static struct nfs4_lockowner *
>> +find_lockowner_str_locked(struct nfs4_client *clp, struct xdr_netobj *owner)
>> +{
>> + struct nfs4_stateowner *so;
>> +
>> + so = find_stateowner_str_locked(clp, owner);
>> + if (!so)
>> + return NULL;
>> + return lockowner(so);
>> +}
>> +
>> static struct nfs4_lockowner *
>> find_lockowner_str(struct nfs4_client *clp, struct xdr_netobj *owner)
>> {
>> @@ -7305,10 +7316,8 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
>> struct nfsd4_release_lockowner *rlockowner = &u->release_lockowner;
>> clientid_t *clid = &rlockowner->rl_clientid;
>> struct nfs4_stateowner *sop;
>> - struct nfs4_lockowner *lo = NULL;
>> + struct nfs4_lockowner *lo;
>> struct nfs4_ol_stateid *stp;
>> - struct xdr_netobj *owner = &rlockowner->rl_owner;
>> - unsigned int hashval = ownerstr_hashval(owner);
>> __be32 status;
>> struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>> struct nfs4_client *clp;
>> @@ -7322,32 +7331,18 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
>> return status;
>>
>> clp = cstate->clp;
>> - /* Find the matching lock stateowner */
>> spin_lock(&clp->cl_lock);
>> - list_for_each_entry(sop, &clp->cl_ownerstr_hashtbl[hashval],
>> - so_strhash) {
>> -
>> - if (sop->so_is_open_owner || !same_owner_str(sop, owner))
>> - continue;
>> -
>> - /* see if there are still any locks associated with it */
>> - lo = lockowner(sop);
>> - list_for_each_entry(stp, &sop->so_stateids, st_perstateowner) {
>> - if (check_for_locks(stp->st_stid.sc_file, lo)) {
>> - status = nfserr_locks_held;
>> - spin_unlock(&clp->cl_lock);
>> - return status;
>> - }
>> - }
>> + sop = find_stateowner_str_locked(clp, &rlockowner->rl_owner);
>> + spin_unlock(&clp->cl_lock);
>> + if (!sop)
>> + return nfs_ok;
>>
>> - nfs4_get_stateowner(sop);
>> - break;
>> - }
>> - if (!lo) {
>> - spin_unlock(&clp->cl_lock);
>> - return status;
>> - }
>> + lo = lockowner(sop);
>> + list_for_each_entry(stp, &sop->so_stateids, st_perstateowner)
>> + if (check_for_locks(stp->st_stid.sc_file, lo))
>> + return nfserr_locks_held;
>>
>
> It has been a while since I was in this code, but is it safe to walk the
> above list without holding the cl_lock?
Shoot.
I thought holding a reference on the stateowner would be enough,
but looks like everyone else holds cl_lock. More chin scratching
and caffeine needed.
>> + spin_lock(&clp->cl_lock);
>> unhash_lockowner_locked(lo);
>> while (!list_empty(&lo->lo_owner.so_stateids)) {
>> stp = list_first_entry(&lo->lo_owner.so_stateids,
>> @@ -7360,8 +7355,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
>> free_ol_stateid_reaplist(&reaplist);
>> remove_blocked_locks(lo);
>> nfs4_put_stateowner(&lo->lo_owner);
>> -
>> - return status;
>> + return nfs_ok;
>> }
>>
>> static inline struct nfs4_client_reclaim *
>>
>>
>
> --
> Jeff Layton <[email protected]>
>
--
Chuck Lever