Re: [PATCH 5/5] btrfs: document extent buffer locking

[David Sterba <dsterba@suse.cz>]

On Tue, Oct 22, 2019 at 12:53:29PM +0300, Nikolay Borisov wrote:
> 
> 
> On 17.10.19 г. 22:39 ч., David Sterba wrote:
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/locking.c | 110 +++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 96 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
> > index e0e0430577aa..2a0e828b4470 100644
> > --- a/fs/btrfs/locking.c
> > +++ b/fs/btrfs/locking.c
> > @@ -13,6 +13,48 @@
> >  #include "extent_io.h"
> >  #include "locking.h"
> >  
> > +/*
> > + * Extent buffer locking
> > + * ~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + * The locks use a custom scheme that allows to do more operations than are
> > + * available fromt current locking primitives. The building blocks are still
> > + * rwlock and wait queues.
> > + *
> > + * Required semantics:
> 
> Thinking out loud here..
> 
> > + *
> > + * - reader/writer exclusion
> 
> RWSEM has that
> 
> > + * - writer/writer exclusion
> 
> RWSEM has that
> 
> > + * - reader/reader sharing
> 
> RWSEM has that
> > + * - spinning lock semantics
> 
> RWSEM has that in the form of optimistic spinning which would be shorter
> than what the custom implementation provides.
> 
> > + * - blocking lock semantics
> 
> RWSEM has that

'blocking' in btrfs lock does not hold the rwlock, does rwsem do it the
same? The term might be confusing, a thread that tries to get a lock is
also blocking (and spinning while trying), but the point here is that
the lock is not held so any sleeping operation can happen. And with the
locking status offloaded, we're not nesting the locking primitives, but
only the big tree locks.

> > + * - try-lock semantics for readers and writers
> 
> down_write_trylock, down_read_trylock.
> 
> > + * - one level nesting, allowing read lock to be taken by the same thread that
> > + *   already has write lock
> 
> this might be somewhat problematic but there is downgrade_write which
> could be used, I'll have to check.

The read lock happens transaprently from the POV of the thread that
takes it. As there's only one known context where the write->read
happens, it would be possible to do some magic to convert it, but it's
still problematic.

The read-function is also called without the write lock. So it would
have to atomically check if the lock is held for write AND by the same
task. I don't know if this is feasible.