Re: [PATCH 2/4] btrfs: inode: remove variable shadowing in btrfs_invalidatepage()

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

On 2020/12/17 下午1:38, Su Yue wrote:
> 
> On Thu 17 Dec 2020 at 12:57, Qu Wenruo <wqu@suse.com> wrote:
> 
>> In btrfs_invalidatepage() we re-declare @tree variable as
>> btrfs_ordered_inode_tree.
>>
>> Remove such variable shadowing which can be very confusing.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  fs/btrfs/inode.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index dced71bccaac..b4d36d138008 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -8169,6 +8169,7 @@ static void btrfs_invalidatepage(struct page 
>> *page, unsigned int offset,
>>                   unsigned int length)
>>  {
>>      struct btrfs_inode *inode = BTRFS_I(page->mapping->host);
>> +    struct btrfs_ordered_inode_tree *ordered_tree = 
>> &inode->ordered_tree;
>>
> Any reason for the declaration here? I didn't find that patch[3/4] use it.

Didn't that ordered_tree get used lines below?

> 
>>      struct extent_io_tree *tree = &inode->io_tree;
>>      struct btrfs_ordered_extent *ordered;
>>      struct extent_state *cached_state = NULL;
>> @@ -8218,15 +8219,11 @@ static void btrfs_invalidatepage(struct page 
>> *page, unsigned int offset,
>>           * for the finish_ordered_io
>>           */
>>          if (TestClearPagePrivate2(page)) {
>> -            struct btrfs_ordered_inode_tree *tree;
>> -
> Better to just rename the @tree to @ordered_tree.

Isn't that exactly what I did?

Thanks,
Qu
> 
>> -            tree = &inode->ordered_tree;
>> -
>> -            spin_lock_irq(&tree->lock);
>> +            spin_lock_irq(&ordered_tree->lock);
>>              set_bit(BTRFS_ORDERED_TRUNCATED, &ordered->flags);
>>              ordered->truncated_len = min(ordered->truncated_len,
>>                      start - ordered->file_offset);
>> -            spin_unlock_irq(&tree->lock);
>> +            spin_unlock_irq(&ordered_tree->lock);
>>
>>              ASSERT(end - start + 1 < U32_MAX);
>>              if (btrfs_dec_test_ordered_pending(inode, &ordered,
>