Skip to contentSkip to conversation

FAILED: patch "[PATCH] btrfs: handle logged extent failure properly" failed to apply to 5.6-stable tree

PATCH [email protected] 2 participants 4 messages Apr 14, 2020 – Apr 16, 2020

Subsystems (2)
Stable branch Btrfs file system
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From ab9b2c7b32e6be53cac2e23f5b2db66815a7d972 Mon Sep 17 00:00:00 2001
From: Josef Bacik <[email protected]>
Date: Thu, 13 Feb 2020 10:47:30 -0500
Subject: [PATCH] btrfs: handle logged extent failure properly

If we're allocating a logged extent we attempt to insert an extent record for the file extent directly. We increase space_info->bytes_reserved, because the extent entry addition will call btrfs_update_block_group(), which will convert the ->bytes_reserved to ->bytes_used. However if we fail at any point while inserting the extent entry we will bail and leave space on ->bytes_reserved, which will trigger a WARN_ON() on umount. Fix this by pinning the space if we fail to insert, which is what happens in every other failure case that involves adding the extent entry.

CC [email protected] # 5.4+
Reviewed-by Johannes Thumshirn <[email protected]>
Reviewed-by Nikolay Borisov <[email protected]>
Reviewed-by Qu Wenruo <[email protected]>
Signed-off-by Josef Bacik <[email protected]>
Reviewed-by David Sterba <[email protected]>
Signed-off-by David Sterba <[email protected]>
fs/btrfs/extent-tree.c +1 -1
index 136fffb76428..7eef91d6c2b6 100644
@@ -4422,7 +4422,7 @@ int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,
 	ret = alloc_reserved_file_extent(trans, 0, root_objectid, 0, owner,
 					 offset, ins, 1);
 	if (ret)
-		btrfs_pin_extent(fs_info, ins->objectid, ins->offset, 1);
+		btrfs_pin_extent(trans, ins->objectid, ins->offset, 1);
 	btrfs_put_block_group(block_group);
 	return ret;
 }
maintainer
#1 lore raw
Quoted 27 lines · On Tue, Apr 14, 2020 at 04:21:45PM +0200, [email protected] wrote:

The patch below does not apply to the 5.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From ab9b2c7b32e6be53cac2e23f5b2db66815a7d972 Mon Sep 17 00:00:00 2001
From: Josef Bacik <[email protected]>
Date: Thu, 13 Feb 2020 10:47:30 -0500
Subject: [PATCH] btrfs: handle logged extent failure properly

If we're allocating a logged extent we attempt to insert an extent record for the file extent directly. We increase space_info->bytes_reserved, because the extent entry addition will call btrfs_update_block_group(), which will convert the ->bytes_reserved to ->bytes_used. However if we fail at any point while inserting the extent entry we will bail and leave space on ->bytes_reserved, which will trigger a WARN_ON() on umount. Fix this by pinning the space if we fail to insert, which is what happens in every other failure case that involves adding the extent entry.

Quoted 7 lines
CC [email protected] # 5.4+
Reviewed-by Johannes Thumshirn <[email protected]>
Reviewed-by Nikolay Borisov <[email protected]>
Reviewed-by Qu Wenruo <[email protected]>
Signed-off-by Josef Bacik <[email protected]>
Reviewed-by David Sterba <[email protected]>
Signed-off-by David Sterba <[email protected]>

Greg, I've noticed that you've fixed it up for 5.5 and 5.4 but no for 5.6? I've queued it up for 5.6 as well.

Signature · 2 lines
Thanks,
Sasha
author maintainer
#2 lore raw
Quoted 36 lines · Sasha Levin
Quoted 27 lines · On Tue, Apr 14, 2020 at 04:21:45PM +0200, [email protected] wrote:

The patch below does not apply to the 5.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From ab9b2c7b32e6be53cac2e23f5b2db66815a7d972 Mon Sep 17 00:00:00 2001

From: Josef Bacik <[email protected]>
Date: Thu, 13 Feb 2020 10:47:30 -0500
Subject: [PATCH] btrfs: handle logged extent failure properly

If we're allocating a logged extent we attempt to insert an extent record for the file extent directly. We increase space_info->bytes_reserved, because the extent entry addition will call btrfs_update_block_group(), which will convert the ->bytes_reserved to ->bytes_used. However if we fail at any point while inserting the extent entry we will bail and leave space on ->bytes_reserved, which will trigger a WARN_ON() on umount. Fix this by pinning the space if we fail to insert, which is what happens in every other failure case that involves adding the extent entry.

Quoted 7 lines
CC [email protected] # 5.4+
Reviewed-by Johannes Thumshirn <[email protected]>
Reviewed-by Nikolay Borisov <[email protected]>
Reviewed-by Qu Wenruo <[email protected]>
Signed-off-by Josef Bacik <[email protected]>
Reviewed-by David Sterba <[email protected]>
Signed-off-by David Sterba <[email protected]>
Quoted 2 lines

Greg, I've noticed that you've fixed it up for 5.5 and 5.4 but no for 5.6? I've queued it up for 5.6 as well.

I didn't include this in 5.5 or 5.4, so please queue it up in those two trees as well.

thanks,

greg k-h

maintainer
#3 lore raw
Quoted 37 lines · Greg KH
Quoted 36 lines · Sasha Levin
Quoted 34 lines · On Tue, Apr 14, 2020 at 04:21:45PM +0200, [email protected] wrote:

The patch below does not apply to the 5.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From ab9b2c7b32e6be53cac2e23f5b2db66815a7d972 Mon Sep 17 00:00:00 2001

From: Josef Bacik <[email protected]>
Date: Thu, 13 Feb 2020 10:47:30 -0500
Subject: [PATCH] btrfs: handle logged extent failure properly

If we're allocating a logged extent we attempt to insert an extent record for the file extent directly. We increase space_info->bytes_reserved, because the extent entry addition will call btrfs_update_block_group(), which will convert the ->bytes_reserved to ->bytes_used. However if we fail at any point while inserting the extent entry we will bail and leave space on ->bytes_reserved, which will trigger a WARN_ON() on umount. Fix this by pinning the space if we fail to insert, which is what happens in every other failure case that involves adding the extent entry.

CC [email protected] # 5.4+
Reviewed-by Johannes Thumshirn <[email protected]>
Reviewed-by Nikolay Borisov <[email protected]>
Reviewed-by Qu Wenruo <[email protected]>
Signed-off-by Josef Bacik <[email protected]>
Reviewed-by David Sterba <[email protected]>
Signed-off-by David Sterba <[email protected]>
Quoted 5 lines
Quoted 2 lines

Greg, I've noticed that you've fixed it up for 5.5 and 5.4 but no for 5.6? I've queued it up for 5.6 as well.

I didn't include this in 5.5 or 5.4, so please queue it up in those two trees as well.

Hm, looking at the patch:

fs/btrfs/extent-tree.c +1 -1
index a7bc66121330e..219ac9990513e 100644
@@ -4431,7 +4431,7 @@ int btrfs_alloc_logged_file_extent(struct btrfs_trans_handle *trans,
       ret = alloc_reserved_file_extent(trans, 0, root_objectid, 0, owner,
                                        offset, ins, 1);
       if (ret)
-               btrfs_pin_extent(fs_info, ins->objectid, ins->offset, 1);
+               btrfs_pin_extent(trans, ins->objectid, ins->offset, 1);
       btrfs_put_block_group(block_group);
       return ret;

}

It changes the var and type used to call btrfs_pin_extent() without changing the declaration of btrfs_pin_extent() itself - which is weird.

The change to btrfs_pin_extent() was done in b25c36f84b59 ("btrfs: Make btrfs_pin_extent take trans handle"), but then I'm confused how this patch on it's own fixes anything but a build issue - which makes the commit message confusing :)

Maybe one of the btrfs folks could clarify?

Signature · 2 lines
Thanks,
Sasha