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

[Su Yue <l@damenly.su>]

On Thu 17 Dec 2020 at 13:42, Qu Wenruo <quwenruo.btrfs@gmx.com> 
wrote:

> 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?

What I mean is that keep the declaration in the block since no 
further use of it.

>
> 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,
>>