Re: [PATCH v5 2/5] btrfs: subpage: introduce helpers to handle subpage delalloc locking

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/5/21 17:20, Naohiro Aota 写道:
[...]
>> +void btrfs_folio_set_writer_lock(const struct btrfs_fs_info *fs_info,
>> +				 struct folio *folio, u64 start, u32 len)
>> +{
>> +	struct btrfs_subpage *subpage;
>> +	unsigned long flags;
>> +	int start_bit;
>> +	int nbits;
>
> May want to use unsigned int for a consistency...
>

I can definitely change all the int to "unsigned int" to be consistent
during pushing to for-next branch.

[...]
>> +	found = true;
>> +	*found_start_ret = folio_pos(folio) +
>> +		((first_set - locked_bitmap_start) << fs_info->sectorsize_bits);
>
> It's a bit fearful to see an "int" value is shifted and added into u64
> value. But, I guess sectorsize is within 32-bit range, right?

In fact, (first_set - locked_bitmap_start) is never going to be larger
than (PAGE_SIZE / sectorsize).

I can add extra ASSERT() to be extra safe for that too.

Thanks,
Qu

>
>> +
>> +	first_zero = find_next_zero_bit(subpage->bitmaps,
>> +					locked_bitmap_end, first_set);
>> +	*found_len_ret = (first_zero - first_set) << fs_info->sectorsize_bits;
>> +out:
>> +	spin_unlock_irqrestore(&subpage->lock, flags);
>> +	return found;
>> +}
>> +
>> +/*
>> + * Unlike btrfs_folio_end_writer_lock() which unlock a specified subpage range,
>> + * this would end all writer locked ranges of a page.
>> + *
>> + * This is for the locked page of __extent_writepage(), as the locked page
>> + * can contain several locked subpage ranges.
>> + */
>> +void btrfs_folio_end_all_writers(const struct btrfs_fs_info *fs_info,
>> +				 struct folio *folio)
>> +{
>> +	u64 folio_start = folio_pos(folio);
>> +	u64 cur = folio_start;
>> +
>> +	ASSERT(folio_test_locked(folio));
>> +	if (!btrfs_is_subpage(fs_info, folio->mapping)) {
>> +		folio_unlock(folio);
>> +		return;
>> +	}
>> +
>> +	while (cur < folio_start + PAGE_SIZE) {
>> +		u64 found_start;
>> +		u32 found_len;
>> +		bool found;
>> +		bool last;
>> +
>> +		found = btrfs_subpage_find_writer_locked(fs_info, folio, cur,
>> +							 &found_start, &found_len);
>> +		if (!found)
>> +			break;
>> +		last = btrfs_subpage_end_and_test_writer(fs_info, folio,
>> +							 found_start, found_len);
>> +		if (last) {
>> +			folio_unlock(folio);
>> +			break;
>> +		}
>> +		cur = found_start + found_len;
>> +	}
>> +}
>> +
>>   #define GET_SUBPAGE_BITMAP(subpage, subpage_info, name, dst)		\
>>   	bitmap_cut(dst, subpage->bitmaps, 0,				\
>>   		   subpage_info->name##_offset, subpage_info->bitmap_nr_bits)
>> diff --git a/fs/btrfs/subpage.h b/fs/btrfs/subpage.h
>> index 4b363d9453af..9f19850d59f2 100644
>> --- a/fs/btrfs/subpage.h
>> +++ b/fs/btrfs/subpage.h
>> @@ -112,6 +112,13 @@ int btrfs_folio_start_writer_lock(const struct btrfs_fs_info *fs_info,
>>   				  struct folio *folio, u64 start, u32 len);
>>   void btrfs_folio_end_writer_lock(const struct btrfs_fs_info *fs_info,
>>   				 struct folio *folio, u64 start, u32 len);
>> +void btrfs_folio_set_writer_lock(const struct btrfs_fs_info *fs_info,
>> +				 struct folio *folio, u64 start, u32 len);
>> +bool btrfs_subpage_find_writer_locked(const struct btrfs_fs_info *fs_info,
>> +				      struct folio *folio, u64 search_start,
>> +				      u64 *found_start_ret, u32 *found_len_ret);
>> +void btrfs_folio_end_all_writers(const struct btrfs_fs_info *fs_info,
>> +				 struct folio *folio);
>>
>>   /*
>>    * Template for subpage related operations.
>> --
>> 2.45.0
>>