Re: [PATCH v2 02/18] btrfs: extent_io: refactor __extent_writepage_io() to improve readability

[Nikolay Borisov <nborisov@suse.com>]

On 10.12.20 г. 14:53 ч., Qu Wenruo wrote:
> 
> 
> On 2020/12/10 下午8:12, Nikolay Borisov wrote:
>>
>>
>> On 10.12.20 г. 8:38 ч., Qu Wenruo wrote:
>>> The refactor involves the following modifications:
>>> - iosize alignment
>>>   In fact we don't really need to manually do alignment at all.
>>>   All extent maps should already be aligned, thus basic ASSERT() check
>>>   would be enough.
>>>
>>> - redundant variables
>>>   We have extra variable like blocksize/pg_offset/end.
>>>   They are all unnecessary.
>>>
>>>   @blocksize can be replaced by sectorsize size directly, and it's only
>>>   used to verify the em start/size is aligned.
>>>
>>>   @pg_offset can be easily calculated using @cur and page_offset(page).
>>>
>>>   @end is just assigned to @page_end and never modified, use @page_end
>>>   to replace it.
>>>
>>> - remove some BUG_ON()s
>>>   The BUG_ON()s are for extent map, which we have tree-checker to check
>>>   on-disk extent data item and runtime check.
>>>   ASSERT() should be enough.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>>  fs/btrfs/extent_io.c | 37 +++++++++++++++++--------------------
>>>  1 file changed, 17 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>>> index 2650e8720394..612fe60b367e 100644
>>> --- a/fs/btrfs/extent_io.c
>>> +++ b/fs/btrfs/extent_io.c
>>> @@ -3515,17 +3515,14 @@ static noinline_for_stack int __extent_writepage_io(struct btrfs_inode *inode,
>>>  				 unsigned long nr_written,
>>>  				 int *nr_ret)
>>>  {
>>> +	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>  	struct extent_io_tree *tree = &inode->io_tree;
>>>  	u64 start = page_offset(page);
>>>  	u64 page_end = start + PAGE_SIZE - 1;
>>
>> nit: page_end should be renamed to end because start now points to the
>> logical logical byte offset, i.e having "page" in the name is misleading.
> 
> But page_offset() along page_end is still logical bytenr, thus I didn't
> see much confusion here...

Exactly page_offset converts the page index to a logical bytenr and that
point we no longer care about the physical page but the logical range
which is PAGE_SIZE. 'page_end' is really some logical affset which spans
a PAGE_SIZE region
> 
> Thanks,
> Qu
>>
>> <snip>
>>
>