| Message ID | 29bc20b873dca2bf2e44af4b13ffb85c28d7b3de.1733989299.git.jth@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: more RST delete fixes | expand |
On Thu, Dec 12, 2024 at 7:56 AM Johannes Thumshirn <jth@kernel.org> wrote: > > From: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > When searching for a RAID stripe-extent for deletion, use an offset of -1 > to always get the "left" slot in the btree and correctly handle the slot > selection. Can you elaborate? It's not clear from this very brief change log what the problem is, or if there's actually a problem. Is this a bug, and what are the consequences? Are we missing stripe extent items and causing some corruption or just unnecessary items? Or is it just an optimization? > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/raid-stripe-tree.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c > index d341fc2a8a4f..d6f7d7d60e76 100644 > --- a/fs/btrfs/raid-stripe-tree.c > +++ b/fs/btrfs/raid-stripe-tree.c > @@ -81,14 +81,18 @@ int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start, u64 le > while (1) { > key.objectid = start; > key.type = BTRFS_RAID_STRIPE_KEY; > - key.offset = 0; > + key.offset = (u64)-1; On a first glance, having the 0 seems to be what leaves us at what you mention in the change log as "the most left slot" and not -1. For example if there's an item with key (X BTRFS_RAID_STRIPE_KEY 1M) and the input arguments are start == X and length == 1M, doing the search with offset 0 leaves at that exact slot where the item is, but with an offset of (u64)-1 it leaves us on the next slot. So please provide an example leaf layout and an example range passed to btrfs_delete_raid_extent() that results in a bug or non-optimal behaviour, or whatever is the problem you found. Thanks. > > ret = btrfs_search_slot(trans, stripe_root, &key, path, -1, 1); > if (ret < 0) > break; > > - if (path->slots[0] == btrfs_header_nritems(path->nodes[0])) > - path->slots[0]--; > + if (ret == 1) { > + ret = 0; > + if (path->slots[0] == > + btrfs_header_nritems(path->nodes[0])) > + path->slots[0]--; > + } > > leaf = path->nodes[0]; > slot = path->slots[0]; > -- > 2.43.0 > >
diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c index d341fc2a8a4f..d6f7d7d60e76 100644 --- a/fs/btrfs/raid-stripe-tree.c +++ b/fs/btrfs/raid-stripe-tree.c @@ -81,14 +81,18 @@ int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start, u64 le while (1) { key.objectid = start; key.type = BTRFS_RAID_STRIPE_KEY; - key.offset = 0; + key.offset = (u64)-1; ret = btrfs_search_slot(trans, stripe_root, &key, path, -1, 1); if (ret < 0) break; - if (path->slots[0] == btrfs_header_nritems(path->nodes[0])) - path->slots[0]--; + if (ret == 1) { + ret = 0; + if (path->slots[0] == + btrfs_header_nritems(path->nodes[0])) + path->slots[0]--; + } leaf = path->nodes[0]; slot = path->slots[0];