mbox series

[v2,0/8] btrfs: zoned: unify relocation on a zoned and regular FS

Message ID cover.1631117101.git.johannes.thumshirn@wdc.com (mailing list archive)
Headers show
Series btrfs: zoned: unify relocation on a zoned and regular FS | expand

Message

Johannes Thumshirn Sept. 8, 2021, 4:19 p.m. UTC
A while ago David reported a bug in zoned btrfs' relocation code. The bug is
triggered because relocation on a zoned filesystem does not preallocate the
extents it copies and a writeback process running in parallel can cause a
split of the written extent. But splitting extents is currently not allowed on
relocation as it assumes a one to one copy of the relocated extents.

This causes transaction aborts and the filessytem switching to read-only in
order to prevent further damage.

The first patch in this series is just a preparation to avoid overly long
lines in follow up patches. Patch number two adds a dedicated block group for
relocation on a zoned filesystem. Number three excludes multiple processes
from adding pages to a relocation inode in a zoned filesystem. Patch four
switches relocation from REQ_OP_ZONE_APPEND to regular REQ_OP_WRITE, five
prepares an ASSERT()ion that we can enter the nocow path on a zoned filesystem
under very special circumstances and the sixth patch then switches the
relocation code for a zoned filesystem to using the same code path as we use
on a non zoned filesystem. As the changes before have made the prerequisites
to do so. The last two patches in this series are just a simple rename of a
function whose name we have twice in the btrfs codebase but with a different
purpose in different files and a cleanup of a complicated boolean comparison
into an if to make it stand out.

Changes to v1:
- Added patches 3 and 8
- Added comments to patches 4 and 5 (Naohiro)
- Move btrfs_clear_data_reloc_bg() out of line (David)
- Untangle the 'skip' assing into an if (David)
- Commented new fs_info members (David)

Johannes Thumshirn (8):
  btrfs: introduce btrfs_is_data_reloc_root
  btrfs: zoned: add a dedicated data relocation block group
  btrfs: zoned: only allow one process to add pages to a relocation
    inode
  btrfs: zoned: use regular writes for relocation
  btrfs: check for relocation inodes on zoned btrfs in should_nocow
  btrfs: zoned: allow preallocation for relocation inodes
  btrfs: rename setup_extent_mapping in relocation code
  btrfs: zoned: let the for_treelog test in the allocator stand out

 fs/btrfs/block-group.c |  1 +
 fs/btrfs/ctree.h       | 12 +++++++++
 fs/btrfs/disk-io.c     |  3 ++-
 fs/btrfs/extent-tree.c | 60 +++++++++++++++++++++++++++++++++++++-----
 fs/btrfs/extent_io.c   |  7 +++++
 fs/btrfs/inode.c       | 29 +++++++++++---------
 fs/btrfs/relocation.c  | 43 +++++-------------------------
 fs/btrfs/zoned.c       | 21 +++++++++++++++
 fs/btrfs/zoned.h       |  4 ++-
 9 files changed, 123 insertions(+), 57 deletions(-)

Comments

David Sterba Sept. 13, 2021, 3:51 p.m. UTC | #1
On Thu, Sep 09, 2021 at 01:19:24AM +0900, Johannes Thumshirn wrote:
> A while ago David reported a bug in zoned btrfs' relocation code. The bug is
> triggered because relocation on a zoned filesystem does not preallocate the
> extents it copies and a writeback process running in parallel can cause a
> split of the written extent. But splitting extents is currently not allowed on
> relocation as it assumes a one to one copy of the relocated extents.
> 
> This causes transaction aborts and the filessytem switching to read-only in
> order to prevent further damage.
> 
> The first patch in this series is just a preparation to avoid overly long
> lines in follow up patches. Patch number two adds a dedicated block group for
> relocation on a zoned filesystem. Number three excludes multiple processes
> from adding pages to a relocation inode in a zoned filesystem. Patch four
> switches relocation from REQ_OP_ZONE_APPEND to regular REQ_OP_WRITE, five
> prepares an ASSERT()ion that we can enter the nocow path on a zoned filesystem
> under very special circumstances and the sixth patch then switches the
> relocation code for a zoned filesystem to using the same code path as we use
> on a non zoned filesystem. As the changes before have made the prerequisites
> to do so. The last two patches in this series are just a simple rename of a
> function whose name we have twice in the btrfs codebase but with a different
> purpose in different files and a cleanup of a complicated boolean comparison
> into an if to make it stand out.
> 
> Changes to v1:
> - Added patches 3 and 8
> - Added comments to patches 4 and 5 (Naohiro)
> - Move btrfs_clear_data_reloc_bg() out of line (David)
> - Untangle the 'skip' assing into an if (David)
> - Commented new fs_info members (David)
> 
> Johannes Thumshirn (8):
>   btrfs: introduce btrfs_is_data_reloc_root
>   btrfs: zoned: add a dedicated data relocation block group
>   btrfs: zoned: only allow one process to add pages to a relocation
>     inode
>   btrfs: zoned: use regular writes for relocation
>   btrfs: check for relocation inodes on zoned btrfs in should_nocow
>   btrfs: zoned: allow preallocation for relocation inodes
>   btrfs: rename setup_extent_mapping in relocation code
>   btrfs: zoned: let the for_treelog test in the allocator stand out

I did a few minor fixups, patches moved from topic branch to misc-next.
Thanks.
Johannes Thumshirn Sept. 14, 2021, 6:58 a.m. UTC | #2
On 13/09/2021 17:51, David Sterba wrote:
> 
> I did a few minor fixups, patches moved from topic branch to misc-next.
> Thanks.
> 

Thanks.