[pnfs] [PATCH] check that the owner is unlocking

[Marc Eshel]

"J. Bruce Fields" <bfields at fieldses.org> wrote on 05/16/2008 10:22:31 AM:

> "J. Bruce Fields" <bfields at fieldses.org> 
> 05/16/2008 10:22 AM
> 
> To
> 
> Marc Eshel <eshel at almaden.ibm.com>
> 
> cc
> 
> pnfs at linux-nfs.org
> 
> Subject
> 
> Re: [pnfs] [PATCH] check that the owner is unlocking
> 
> On Fri, May 16, 2008 at 10:12:33AM -0700, Marc Eshel wrote:
> > "J. Bruce Fields" <bfields at fieldses.org> wrote on 05/16/2008 09:59:35 
AM:
> > > Right, but I don't understand why the client_mutex_owner is needed, 
when
> > > the struct mutex already has an owner field, and already uses it to 
do
> > > the above check for us; see include/linux/mutex.h.  Looks like you 
need
> > > CONFIG_DEBUG_MUTEXES defined (under Kernel Hacking->"Mutex 
debugging:
> > > basic checks").
> > 
> > I am not sure if there is better way to implement 
> > nfs4_lock_state_nested(),
> 
> Again, just look at the definition of struct mutex in
> include/linux/mutex.h.  You can change the
> 
>    if (client_mutex_owner == current_thread_info())
> 
> to
> 
>    if (client_mutex.owner == current_thread_info())
> 
> and make NFS_V4_1 depend on CONFIG_DEBUG_MUTEXES.

I don't think that it is reasonable to depend on CONFIG_DEBUG_MUTEXES and 
it not only 4.2 delegation code is using nfs4_lock_state_nested() too.

> 
> In the longer term, I think we should work out how to get rid of
> nfs4_lock_state_nested().

Yes.

> 
> --b.
> 
> > I did not this routine but as long as this 
> > routine is there it depend on that owner field.
> > Marc.
> > 
> > > 
> > > --b.
> > > 
> > > > so if some thread unlocks 
> > > > a lock that it doesn't own it will reset the real owner to NULL 
which 
> > will 
> > > > confuse functions like nfs4_lock_state_nested().
> >