The use of the st_mutex has been confusing the validator. Use the
proper nested notation so as to not produce warnings.
Signed-off-by: Andrew Elble <[email protected]>
---
v2: added mutex_lock_nested to init_lock_stateid() for consistency
fs/nfsd/nfs4state.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0d98d73bd84e..62909f3947b2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3548,7 +3548,7 @@ static void nfs4_free_openowner(struct nfs4_stateowner *so)
{
__be32 ret;
- mutex_lock(&stp->st_mutex);
+ mutex_lock_nested(&stp->st_mutex, 1);
ret = nfsd4_verify_open_stid(&stp->st_stid);
if (ret != nfs_ok)
mutex_unlock(&stp->st_mutex);
@@ -3612,7 +3612,7 @@ static void nfs4_free_openowner(struct nfs4_stateowner *so)
stp = open->op_stp;
/* We are moving these outside of the spinlocks to avoid the warnings */
mutex_init(&stp->st_mutex);
- mutex_lock(&stp->st_mutex);
+ mutex_lock_nested(&stp->st_mutex, 0);
retry:
spin_lock(&oo->oo_owner.so_client->cl_lock);
@@ -5692,7 +5692,7 @@ static void nfs4_free_lockowner(struct nfs4_stateowner *sop)
struct nfs4_ol_stateid *retstp;
mutex_init(&stp->st_mutex);
- mutex_lock(&stp->st_mutex);
+ mutex_lock_nested(&stp->st_mutex, 0);
retry:
spin_lock(&clp->cl_lock);
spin_lock(&fp->fi_lock);
--
1.8.3.1
On Tue, Nov 07, 2017 at 07:57:26PM -0500, Andrew Elble wrote:
> The use of the st_mutex has been confusing the validator. Use the
> proper nested notation so as to not produce warnings.
Looking around, the usual pattern for simple nesting seems to be to use
just mutex_lock() for the outer lock (equivalent to
mutex_lock_nested(0)), and mutex_lock_nested(., SINGLE_DEPTH_NESTING)
for the inner lock.
Or we could define a new LOCK_STATEID_MUTEX, assuming the rule here is
"lock stateid's are locked after open stateid's". Just a question of
might be simpler to understand.
--b.
>
> Signed-off-by: Andrew Elble <[email protected]>
> ---
> v2: added mutex_lock_nested to init_lock_stateid() for consistency
>
> fs/nfsd/nfs4state.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 0d98d73bd84e..62909f3947b2 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -3548,7 +3548,7 @@ static void nfs4_free_openowner(struct nfs4_stateowner *so)
> {
> __be32 ret;
>
> - mutex_lock(&stp->st_mutex);
> + mutex_lock_nested(&stp->st_mutex, 1);
> ret = nfsd4_verify_open_stid(&stp->st_stid);
> if (ret != nfs_ok)
> mutex_unlock(&stp->st_mutex);
> @@ -3612,7 +3612,7 @@ static void nfs4_free_openowner(struct nfs4_stateowner *so)
> stp = open->op_stp;
> /* We are moving these outside of the spinlocks to avoid the warnings */
> mutex_init(&stp->st_mutex);
> - mutex_lock(&stp->st_mutex);
> + mutex_lock_nested(&stp->st_mutex, 0);
>
> retry:
> spin_lock(&oo->oo_owner.so_client->cl_lock);
> @@ -5692,7 +5692,7 @@ static void nfs4_free_lockowner(struct nfs4_stateowner *sop)
> struct nfs4_ol_stateid *retstp;
>
> mutex_init(&stp->st_mutex);
> - mutex_lock(&stp->st_mutex);
> + mutex_lock_nested(&stp->st_mutex, 0);
> retry:
> spin_lock(&clp->cl_lock);
> spin_lock(&fp->fi_lock);
> --
> 1.8.3.1
"J. Bruce Fields" <[email protected]> writes:
> On Tue, Nov 07, 2017 at 07:57:26PM -0500, Andrew Elble wrote:
>> The use of the st_mutex has been confusing the validator. Use the
>> proper nested notation so as to not produce warnings.
>
> Looking around, the usual pattern for simple nesting seems to be to use
> just mutex_lock() for the outer lock (equivalent to
> mutex_lock_nested(0)), and mutex_lock_nested(., SINGLE_DEPTH_NESTING)
> for the inner lock.
>
> Or we could define a new LOCK_STATEID_MUTEX, assuming the rule here is
> "lock stateid's are locked after open stateid's". Just a question of
> might be simpler to understand.
I'm okay with whatever you think is best here - my thought was that the
mutex_lock_nested(0) called more attention to how it was working given
that acquiring that lock class the second time is now a little bit more
hidden in nfsd4_lock_ol_stateid().
Thanks,
Andy
--
Andrew W. Elble
[email protected]
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912
On Wed, Nov 08, 2017 at 04:42:14PM -0500, Andrew W Elble wrote:
> "J. Bruce Fields" <[email protected]> writes:
>
> > On Tue, Nov 07, 2017 at 07:57:26PM -0500, Andrew Elble wrote:
> >> The use of the st_mutex has been confusing the validator. Use the
> >> proper nested notation so as to not produce warnings.
> >
> > Looking around, the usual pattern for simple nesting seems to be to use
> > just mutex_lock() for the outer lock (equivalent to
> > mutex_lock_nested(0)), and mutex_lock_nested(., SINGLE_DEPTH_NESTING)
> > for the inner lock.
> >
> > Or we could define a new LOCK_STATEID_MUTEX, assuming the rule here is
> > "lock stateid's are locked after open stateid's". Just a question of
> > might be simpler to understand.
>
> I'm okay with whatever you think is best here - my thought was that the
> mutex_lock_nested(0) called more attention to how it was working given
> that acquiring that lock class the second time is now a little bit more
> hidden in nfsd4_lock_ol_stateid().
I think I'd go for constants like OPEN_STATEID_MUTEX and
LOCK_STATEID_MUTEX in that case. Maybe mild overkill, but it should
explain what's going on?
--b.