Re: [PATCH 0/3] btrfs: make subpage reader/writer counter to be sector aware

[Boris Burkov <boris@bur.io>]

On Wed, Feb 14, 2024 at 02:34:33PM +1030, Qu Wenruo wrote:
> This can be fetched from github, and the branch would be utilized for
> all newer subpage delalloc update to support full sector sized
> compression and zoned:
> https://github.com/adam900710/linux/tree/subpage_delalloc
> 
> Currently we just trace subpage reader/writer counter using an atomic.
> 
> It's fine for the current subpage usage, but for the future, we want to
> be aware of which subpage sector is locked inside a page, for proper
> compression (we only support full page compression for now) and zoned support.

The logic of the patches seems good and self-consistent to me, I don't
see any issues.

However, I think it would be helpful to at least see the client code to
motivate the bitmap a bit more for the ignorant :)

Also, from a semi-cursory inspection, it looks like this relies on
extent locking to ensure that multiple threads don't collide on the
subpage bitmap, is that correct? You should check with Josef that his
plans with getting rid of the extent locking don't clash with this.

Thanks,
Boris

> 
> So here we introduce a new bitmap, called locked bitmap, to trace which
> sector is locked for read/write.
> 
> And since reader/writer are both exclusive (to each other and to the same
> type of lock), we can safely use the same bitmap for both reader and
> writer.
> 
> In theory we can use the bitmap (the weight of the locked bitmap) to
> indicate how many bytes are under reader/write lock, but it's not
> possible yet:
> 
> - No weight support for bitmap range
>   The bitmap API only provides bitmap_weight(), which always starts at
>   bit 0.
> 
> - Need to distinguish read/write lock
> 
> Thus we still keep the reader/writer atomic counter.
> 
> Qu Wenruo (3):
>   btrfs: unexport btrfs_subpage_start_writer() and
>     btrfs_subpage_end_and_test_writer()
>   btrfs: subpage: make reader lock to utilize bitmap
>   btrfs: subpage: make writer lock to utilize bitmap
> 
>  fs/btrfs/subpage.c | 70 ++++++++++++++++++++++++++++++++++++----------
>  fs/btrfs/subpage.h | 16 +++++++----
>  2 files changed, 66 insertions(+), 20 deletions(-)
> 
> -- 
> 2.43.1
>