On Wed, Oct 23, 2019 at 02:16:26PM +0300, Nikolay Borisov wrote:
> > void btrfs_set_lock_blocking_read(struct extent_buffer *eb)
> > {
>
> I think for this and it's write counterpart a
> lockdep_assert_held(eb->lock) will be a better way to document the fact
> the lock needs to be held when those functions are called.
Ok, will add it.
> > trace_btrfs_set_lock_blocking_read(eb);
>
> <snip>
>
> > /*
> > - * drop a blocking read lock
> > + * Release read lock, previously set to blocking by a pairing call to
> > + * btrfs_set_lock_blocking_read(). Can be nested in write lock by the same
> > + * thread.
> > + *
> > + * State of rwlock is unchanged, last reader wakes waiting threads.
> > */
> > void btrfs_tree_read_unlock_blocking(struct extent_buffer *eb)
> > {
> > @@ -279,8 +354,10 @@ void btrfs_tree_read_unlock_blocking(struct extent_buffer *eb)
> > }
> >
> > /*
> > - * take a spinning write lock. This will wait for both
> > - * blocking readers or writers
> > + * Lock for write. Wait for all blocking and spinning readers and writers. This
> > + * starts context where reader lock could be nested by the same thread.
>
> Imo you shouldn't ommit the explicit mention this takes a spinning lock.
But
> > + * The rwlock is held for write upon exit.
the next line in the commit says that. There's no explicit 'spinning'
but I find it sufficient. Feel free to suggest better wording.