diff mbox series

btrfs: qgroup: Fix the wrong io_tree when freeing reserved data space

Message ID 20190912011306.14858-1-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: qgroup: Fix the wrong io_tree when freeing reserved data space | expand

Commit Message

Qu Wenruo Sept. 12, 2019, 1:13 a.m. UTC
Commit bc42bda22345 ("btrfs: qgroup: Fix qgroup reserved space underflow by
only freeing reserved ranges") is freeing wrong range in
BTRFS_I()->io_failure_tree, which should be BTRFS_I()->io_tree.

Just fix it.

Reported-by: Josef Bacik <josef@toxicpanda.com>
Fixes: bc42bda22345 ("btrfs: qgroup: Fix qgroup reserved space underflow by only freeing reserved ranges")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Filipe Manana Sept. 12, 2019, 7:53 a.m. UTC | #1
On Thu, Sep 12, 2019 at 2:31 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Commit bc42bda22345 ("btrfs: qgroup: Fix qgroup reserved space underflow by
> only freeing reserved ranges") is freeing wrong range in
> BTRFS_I()->io_failure_tree, which should be BTRFS_I()->io_tree.

I think you meant wrong tree and not wrong range, since the code
doesn't change the range, only the target tree.

Also, for the sake of completeness, and no matter how obvious you
think it is, can you explicitly mention what's the consequence? I
presume it's a qgroup reserved space leak or underflow.

Thanks.

>
> Just fix it.
>
> Reported-by: Josef Bacik <josef@toxicpanda.com>
> Fixes: bc42bda22345 ("btrfs: qgroup: Fix qgroup reserved space underflow by only freeing reserved ranges")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/qgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 2891b57b9e1e..64bdc3e3652d 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -3492,7 +3492,7 @@ static int qgroup_free_reserved_data(struct inode *inode,
>                  * EXTENT_QGROUP_RESERVED, we won't double free.
>                  * So not need to rush.
>                  */
> -               ret = clear_record_extent_bits(&BTRFS_I(inode)->io_failure_tree,
> +               ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree,
>                                 free_start, free_start + free_len - 1,
>                                 EXTENT_QGROUP_RESERVED, &changeset);
>                 if (ret < 0)
> --
> 2.23.0
>
Qu Wenruo Sept. 12, 2019, 8:32 a.m. UTC | #2
On 2019/9/12 下午3:53, Filipe Manana wrote:
> On Thu, Sep 12, 2019 at 2:31 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> Commit bc42bda22345 ("btrfs: qgroup: Fix qgroup reserved space underflow by
>> only freeing reserved ranges") is freeing wrong range in
>> BTRFS_I()->io_failure_tree, which should be BTRFS_I()->io_tree.
> 
> I think you meant wrong tree and not wrong range, since the code
> doesn't change the range, only the target tree.

Right, wrong tree.

> 
> Also, for the sake of completeness, and no matter how obvious you
> think it is, can you explicitly mention what's the consequence? I
> presume it's a qgroup reserved space leak or underflow.

Yes, qgroup reserved space leak for some error path.

I'll enhance the comment, and also craft a test case for it.

Thanks,
Qu

> 
> Thanks.
> 
>>
>> Just fix it.
>>
>> Reported-by: Josef Bacik <josef@toxicpanda.com>
>> Fixes: bc42bda22345 ("btrfs: qgroup: Fix qgroup reserved space underflow by only freeing reserved ranges")
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  fs/btrfs/qgroup.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index 2891b57b9e1e..64bdc3e3652d 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -3492,7 +3492,7 @@ static int qgroup_free_reserved_data(struct inode *inode,
>>                  * EXTENT_QGROUP_RESERVED, we won't double free.
>>                  * So not need to rush.
>>                  */
>> -               ret = clear_record_extent_bits(&BTRFS_I(inode)->io_failure_tree,
>> +               ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree,
>>                                 free_start, free_start + free_len - 1,
>>                                 EXTENT_QGROUP_RESERVED, &changeset);
>>                 if (ret < 0)
>> --
>> 2.23.0
>>
> 
>
diff mbox series

Patch

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 2891b57b9e1e..64bdc3e3652d 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -3492,7 +3492,7 @@  static int qgroup_free_reserved_data(struct inode *inode,
 		 * EXTENT_QGROUP_RESERVED, we won't double free.
 		 * So not need to rush.
 		 */
-		ret = clear_record_extent_bits(&BTRFS_I(inode)->io_failure_tree,
+		ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree,
 				free_start, free_start + free_len - 1,
 				EXTENT_QGROUP_RESERVED, &changeset);
 		if (ret < 0)