[Cluster-devel] Re: [PATCH 2/2] dlm: Add down/up_write_non_owner to keep lockdep happy

[David Teigland]

On Thu, Nov 12, 2009 at 05:45:39PM +0100, Peter Zijlstra wrote:
> On Thu, 2009-11-12 at 11:14 -0600, David Teigland wrote:
> > up_write_non_owner()
> > addresses this trace, which as you say, is from doing the down and up from
> > different threads (which is the intention): 
> 
> That's really something I cannot advice to do. Aside from loosing
> lock-dependency validation (not a good thing), asymmetric locking like
> that is generally very hard to analyze since its not clear who 'owns'
> what data when.
> 
> There are a few places in the kernel that use the non_owner things, and
> we should generally strive to remove them, not add more.
> 
> Please consider solving your problem without adding things like this.

It's an unusual use case; this lock is not protecting data in a direct
sense, but is controlling who can run in the dlm.

During normal activity, many threads are running through the dlm (any
process accessing the fs, dlm kernel threads, gfs kernel threads), they
all take the read lock when they enter and release it when they exit.

When dlm recovery needs to happen, this lock is taken in write mode to
wait for all the normal, active threads to exit the dlm, and then block
any further access to the dlm until recovery is complete.  Recovery is
initiated by a userland process which takes the write lock.  Recovery is
then performed by the dlm_recoverd thread which releases the write lock
when it's done, and normal activity resumes.  The release from
dlm_recoverd would use up_write_non_owner() to avoid the warning.

I'm sure there are other ways to do this, but I don't know when I'll have
the time to look into them.  In the mean time, the rw_semaphore is simple
and effective.  (I don't mind the warning myself, but others seem to be
annoyed by it.)

Dave