| Message ID | e996ff4c30ef83f271f2495d700635287d5587d9.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 8:06 AM Johannes Thumshirn <jth@kernel.org> wrote: > > From: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > When deleting the front of a RAID stripe-extent the delete code > miscalculates the size on how much to pad the remaining extent part in the > front. > > Fix the calculation so we're always having the sizes we expect. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/raid-stripe-tree.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c > index d6f7d7d60e76..092e24e1de32 100644 > --- a/fs/btrfs/raid-stripe-tree.c > +++ b/fs/btrfs/raid-stripe-tree.c > @@ -138,10 +138,13 @@ int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start, u64 le > * length to the new size and then re-insert the item. > */ > if (found_end > end) { > - u64 diff = found_end - end; > + u64 diff_end = found_end - end; > > btrfs_partially_delete_raid_extent(trans, path, &key, > - diff, diff); > + key.offset - length, > + length); > + ASSERT(key.offset - diff_end == length); > + length = 0; > break; What's the length = 0 for? We break out of the loop right after the assignment and don't use length anymore. Otherwise it looks good: Reviewed-by: Filipe Manana <fdmanana@suse.com> Thanks. > } > > -- > 2.43.0 > >
On 17.12.24 16:03, Filipe Manana wrote:
btrfs_partially_delete_raid_extent(trans,
path, &key,
>> - diff, diff);
>> + key.offset - length,
>> + length);
>> + ASSERT(key.offset - diff_end == length);
>> + length = 0;
>> break;
>
> What's the length = 0 for? We break out of the loop right after the
> assignment and don't use length anymore.
I originally planned on factoring out the loop body into a 'delete one
extent' function and thus I've set length to 0 here so I know we're
breaking out of the loop.
But then I stared too long at this code and decided to not do the
refactoring in this series. Forgot to remove the line though.
diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c index d6f7d7d60e76..092e24e1de32 100644 --- a/fs/btrfs/raid-stripe-tree.c +++ b/fs/btrfs/raid-stripe-tree.c @@ -138,10 +138,13 @@ int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start, u64 le * length to the new size and then re-insert the item. */ if (found_end > end) { - u64 diff = found_end - end; + u64 diff_end = found_end - end; btrfs_partially_delete_raid_extent(trans, path, &key, - diff, diff); + key.offset - length, + length); + ASSERT(key.offset - diff_end == length); + length = 0; break; }