| Message ID | cover.1734514696.git.wqu@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: migrate to "block size" to describe the | expand |
On Wed, Dec 18, 2024 at 08:11:16PM +1030, Qu Wenruo wrote: > [IMPEMENTATION] > To reduce the confusion, this patchset will do such a huge migration in > different steps: With some many changes everywhere this is going to make backports even more tedious. I think it would be best to do the conversion gradually or selectively to code that does not change so often. As you've split it to files we can first pick a few obvious ones (like file-item.c) or gather stats from stable.git. A quick and rough estimate for all 6.x.y releases counting file backports in the individual patches in the list below. This is period of 2 years, 104 weeks (roughly matching number of releases). So if there are 88 patches touching inode.c that's quite likely a conflict in every backport. This should be also correlated agains number of 'sectorsize' per file, so it many not be that bad, for example in ioctl.c there are only 4 occurences so that's fine. The point is we can't do the renames without some sensibility and respect to backports. 88 inode.c 62 disk-io.c 61 volumes.c 57 extent_io.c 57 block-group.c 56 ioctl.c 52 extent-tree.c 49 zoned.c 49 qgroup.c 37 send.c 36 super.c 33 ctree.c 30 transaction.c 29 file.c 28 tree-log.c 28 free-space-cache.c 26 scrub.c 24 ctree.h 22 relocation.c 19 delayed-inode.c 17 tree-checker.c 17 backref.c 15 space-info.c 14 extent_map.c 12 free-space-tree.c 12 disk-io.h 11 block-group.h 11 bio.c 10 ordered-data.c 10 block-rsv.c 9 ref-verify.c 9 file-item.c 9 btrfs_inode.h 8 include/uapi/linux/btrfs.h 8 fs.h 8 delayed-ref.c 8 delalloc-space.c 7 root-tree.c 7 print-tree.c 7 dir-item.c 7 dev-replace.c 7 block-rsv.h 6 sysfs.c 6 extent_io.h 6 defrag.c 6 compression.c 5 volumes.h 5 transaction.h 5 qgroup.h 5 export.c 4 zoned.h 4 space-info.h 4 reflink.c 4 inode-item.c 4 include/trace/events/btrfs.h 4 discard.c 4 delayed-ref.h 3 tree-log.h 3 tree-defrag.c 3 subpage.c 3 raid56.c 3 free-space-tree.h 3 extent-io-tree.c 3 extent-io-tests.c 3 backref.h 2 zlib.c 2 xattr.c 2 uuid-tree.c 2 tree-mod-log.c 2 tests/inode-tests.c 2 tests/extent-map-tests.c 2 tests/extent-buffer-tests.c 2 root-tree.h 2 relocation.h 2 rcu-string.h 2 qgroup-tests.c 2 lzo.c 2 locking.c 2 inode-item.h 2 free-space-cache.h 2 extent-tree.h 2 delayed-inode.h 1 tree-checker.h 1 tests/free-space-tree-tests.c 1 sysfs.h 1 props.c 1 ordered-data.h 1 messages.h 1 messages.c 1 include/uapi/linux/btrfs_tree.h 1 fs.c 1 file-item.h 1 extent_map.h 1 export.h 1 compression.h 1 btrfs-tests.c 1 btrfs.h 1 bio.h
在 2024/12/19 05:01, David Sterba 写道: > On Wed, Dec 18, 2024 at 08:11:16PM +1030, Qu Wenruo wrote: >> [IMPEMENTATION] >> To reduce the confusion, this patchset will do such a huge migration in >> different steps: > > With some many changes everywhere this is going to make backports even > more tedious. I think it would be best to do the conversion gradually or > selectively to code that does not change so often. As you've split it to > files we can first pick a few obvious ones (like file-item.c) or gather > stats from stable.git. For the backports, we can put the btrfs_fs_info union to older kernels and call it a day without the remaining part. So that both newer and older terminology can co-exist without any conflicts. Although backport conflicts will still happen in the context. > > A quick and rough estimate for all 6.x.y releases counting file > backports in the individual patches in the list below. This is period of > 2 years, 104 weeks (roughly matching number of releases). So if there > are 88 patches touching inode.c that's quite likely a conflict in every > backport. > > This should be also correlated agains number of 'sectorsize' per file, > so it many not be that bad, for example in ioctl.c there are only 4 > occurences so that's fine. The point is we can't do the renames without > some sensibility and respect to backports. But at least, when a backport fails, we can easily fix the conflict. It will always be sectorsize -> blocksize, either in the context or the modified lines. Thanks, Qu > > 88 inode.c > 62 disk-io.c > 61 volumes.c > 57 extent_io.c > 57 block-group.c > 56 ioctl.c > 52 extent-tree.c > 49 zoned.c > 49 qgroup.c > 37 send.c > 36 super.c > 33 ctree.c > 30 transaction.c > 29 file.c > 28 tree-log.c > 28 free-space-cache.c > 26 scrub.c > 24 ctree.h > 22 relocation.c > 19 delayed-inode.c > 17 tree-checker.c > 17 backref.c > 15 space-info.c > 14 extent_map.c > 12 free-space-tree.c > 12 disk-io.h > 11 block-group.h > 11 bio.c > 10 ordered-data.c > 10 block-rsv.c > 9 ref-verify.c > 9 file-item.c > 9 btrfs_inode.h > 8 include/uapi/linux/btrfs.h > 8 fs.h > 8 delayed-ref.c > 8 delalloc-space.c > 7 root-tree.c > 7 print-tree.c > 7 dir-item.c > 7 dev-replace.c > 7 block-rsv.h > 6 sysfs.c > 6 extent_io.h > 6 defrag.c > 6 compression.c > 5 volumes.h > 5 transaction.h > 5 qgroup.h > 5 export.c > 4 zoned.h > 4 space-info.h > 4 reflink.c > 4 inode-item.c > 4 include/trace/events/btrfs.h > 4 discard.c > 4 delayed-ref.h > 3 tree-log.h > 3 tree-defrag.c > 3 subpage.c > 3 raid56.c > 3 free-space-tree.h > 3 extent-io-tree.c > 3 extent-io-tests.c > 3 backref.h > 2 zlib.c > 2 xattr.c > 2 uuid-tree.c > 2 tree-mod-log.c > 2 tests/inode-tests.c > 2 tests/extent-map-tests.c > 2 tests/extent-buffer-tests.c > 2 root-tree.h > 2 relocation.h > 2 rcu-string.h > 2 qgroup-tests.c > 2 lzo.c > 2 locking.c > 2 inode-item.h > 2 free-space-cache.h > 2 extent-tree.h > 2 delayed-inode.h > 1 tree-checker.h > 1 tests/free-space-tree-tests.c > 1 sysfs.h > 1 props.c > 1 ordered-data.h > 1 messages.h > 1 messages.c > 1 include/uapi/linux/btrfs_tree.h > 1 fs.c > 1 file-item.h > 1 extent_map.h > 1 export.h > 1 compression.h > 1 btrfs-tests.c > 1 btrfs.h > 1 bio.h
On Thu, Dec 19, 2024 at 06:43:38AM +1030, Qu Wenruo wrote: > > > 在 2024/12/19 05:01, David Sterba 写道: > > On Wed, Dec 18, 2024 at 08:11:16PM +1030, Qu Wenruo wrote: > >> [IMPEMENTATION] > >> To reduce the confusion, this patchset will do such a huge migration in > >> different steps: > > > > With some many changes everywhere this is going to make backports even > > more tedious. I think it would be best to do the conversion gradually or > > selectively to code that does not change so often. As you've split it to > > files we can first pick a few obvious ones (like file-item.c) or gather > > stats from stable.git. > > For the backports, we can put the btrfs_fs_info union to older kernels > and call it a day without the remaining part. > > So that both newer and older terminology can co-exist without any conflicts. > > Although backport conflicts will still happen in the context. This is what I'm concerned about. If patches don't apply cleanly they're rejected from the automated stable workflow and have to be handled manually. > > A quick and rough estimate for all 6.x.y releases counting file > > backports in the individual patches in the list below. This is period of > > 2 years, 104 weeks (roughly matching number of releases). So if there > > are 88 patches touching inode.c that's quite likely a conflict in every > > backport. > > > > This should be also correlated agains number of 'sectorsize' per file, > > so it many not be that bad, for example in ioctl.c there are only 4 > > occurences so that's fine. The point is we can't do the renames without > > some sensibility and respect to backports. > > But at least, when a backport fails, we can easily fix the conflict. > It will always be sectorsize -> blocksize, either in the context or the > modified lines. Easily yes, but multiply that by number of failed patches and number of stable trees to backport. This takes time and currently the upstream backports are best-effort only, not all patches that fail to apply get manually backported. Enough that there are CVEs of everything and we have to care about them at $job so I'm not going to create more problems with backports than we already have just to fix a naming inconvenience.
在 2024/12/19 07:06, David Sterba 写道: > On Thu, Dec 19, 2024 at 06:43:38AM +1030, Qu Wenruo wrote: >> >> >> 在 2024/12/19 05:01, David Sterba 写道: >>> On Wed, Dec 18, 2024 at 08:11:16PM +1030, Qu Wenruo wrote: >>>> [IMPEMENTATION] >>>> To reduce the confusion, this patchset will do such a huge migration in >>>> different steps: >>> >>> With some many changes everywhere this is going to make backports even >>> more tedious. I think it would be best to do the conversion gradually or >>> selectively to code that does not change so often. As you've split it to >>> files we can first pick a few obvious ones (like file-item.c) or gather >>> stats from stable.git. >> >> For the backports, we can put the btrfs_fs_info union to older kernels >> and call it a day without the remaining part. >> >> So that both newer and older terminology can co-exist without any conflicts. >> >> Although backport conflicts will still happen in the context. > > This is what I'm concerned about. If patches don't apply cleanly they're > rejected from the automated stable workflow and have to be handled > manually. > >>> A quick and rough estimate for all 6.x.y releases counting file >>> backports in the individual patches in the list below. This is period of >>> 2 years, 104 weeks (roughly matching number of releases). So if there >>> are 88 patches touching inode.c that's quite likely a conflict in every >>> backport. >>> >>> This should be also correlated agains number of 'sectorsize' per file, >>> so it many not be that bad, for example in ioctl.c there are only 4 >>> occurences so that's fine. The point is we can't do the renames without >>> some sensibility and respect to backports. >> >> But at least, when a backport fails, we can easily fix the conflict. >> It will always be sectorsize -> blocksize, either in the context or the >> modified lines. > > Easily yes, but multiply that by number of failed patches and number of > stable trees to backport. This takes time and currently the upstream > backports are best-effort only, not all patches that fail to apply get > manually backported. Enough that there are CVEs of everything and we > have to care about them at $job so I'm not going to create more problems > with backports than we already have just to fix a naming inconvenience. > But I have to argue, that will only leave the problem untouched forever (even if it's just a naming scheme problem). Yes, it will cause backport problems, but as you also mentioned, the frequency should not be that high (ssunless it's scrub/raid56). With the compatible union, it should further reduce the conflict chance. We have leave the naming problem untouched for over a decade, I think it's time to finish it once for all. And I'm pretty happy to handle the backports if it's just the renaming causing the conflicts. I also thought about the more progressive solution, just put the union into all involved structures, and call it a day with future usage migrated to blocksize. But it's really no different than the current one backport wise, every time a sectorsize is changed to blocksize, future backport will be affected. It's just changing the short huge pain into a long small pain, the amount of pain is not changed. And there will be a way longer window where we have mixed terminology, causing more chaos and confusion. Thus I recommend to take this chance to solve our long existing problems. Or it will really never be fixed. Thanks, Qu
[BACKGROUND] Since day 1, btrfs uses a different terminology, sector size, to describe the minimum data block size. Meanwhile all other filesystems (ext4, xfs, jfs, bcachefs, reiserfs) are using "block size" to describe the same unit. Furthermore, kernel itself has its own sector size, fixed to 512 bytes, as the minimal block IO unit. This will cause extra confusiong when we're talking with other MM/FS developers. [IMPEMENTATION] To reduce the confusion, this patchset will do such a huge migration in different steps: - Introduce btrfs_fs_info::blocksize as an alias for btrfs_fs_info::sectorsize This is done by introducing an anonymous union, allowing @sectorsize and @blocksize to share the same value, and do the migration on a per-file basis. This covers @blocsize, @blocksize_bits, @blocks_per_page. - Rename involved users, comments and local variables - Rename related function names, parameters and even some macros This affects scrub and raid56 most, as I follow the strict "sector" terminology when reworking on those functionality. Thus they will receive very heavy renames. - Finish the btrfs_fs_info::sectorsize rename - Add btrfs_super_block::blocksize as an alias for sectorsize - Add btrfs_ioctl_fs_info_args::blocksize as an alias for sectorsize Unlike the btrfs_fs_info::blocksize change which is purely inside btrfs, those structures are exported as on-disk format and ioctl parameters. To keep the compatibility for older programs which may never update their code, make @blocksize and @sectorsize co-exist as unions, to avoid breakage for older programs. Qu Wenruo (18): btrfs: rename btrfs_fs_info::sectorsize to blocksize for disk-io.c btrfs: migrate subpage.[ch] to use block size terminology btrfs: migrate tree-checker.c to use block size terminology btrfs: migrate scrub.c to use block size terminology btrfs: migrate extent_io.[ch] to use block size terminology btrfs: migrate compression related code to use block size terminology btrfs: migrate free space cache code to use block size terminology btrfs: migrate file-item.c to use block size terminology btrfs: migrate file.c to use block size terminology btrfs: migrate inode.c and btrfs_inode.h to use block size terminology btrfs: migrate raid56.[ch] to use block size terminology btrfs: migrate defrag.c to use block size terminology btrfs: migrate bio.[ch] to use block size terminology btrfs: migrate the remaining sector size users to use block size terminology btrfs: migrate selftests to use block size terminology btrfs: finish the rename of btrfs_fs_info::sectorsize btrfs: migrate btrfs_super_block::sectorsize to blocksize btrfs: migrate the ioctl interfaces to use block size terminology fs/btrfs/accessors.h | 6 +- fs/btrfs/bio.c | 24 +- fs/btrfs/bio.h | 4 +- fs/btrfs/block-group.c | 4 +- fs/btrfs/btrfs_inode.h | 2 +- fs/btrfs/compression.c | 30 +- fs/btrfs/defrag.c | 52 +- fs/btrfs/delalloc-space.c | 26 +- fs/btrfs/delayed-inode.c | 2 +- fs/btrfs/delayed-ref.c | 12 +- fs/btrfs/delayed-ref.h | 4 +- fs/btrfs/dev-replace.c | 12 +- fs/btrfs/direct-io.c | 6 +- fs/btrfs/disk-io.c | 82 +-- fs/btrfs/extent-tree.c | 14 +- fs/btrfs/extent_io.c | 124 ++-- fs/btrfs/extent_io.h | 16 +- fs/btrfs/extent_map.h | 2 +- fs/btrfs/fiemap.c | 6 +- fs/btrfs/file-item.c | 94 +-- fs/btrfs/file.c | 138 ++-- fs/btrfs/free-space-cache.c | 8 +- fs/btrfs/free-space-tree.c | 28 +- fs/btrfs/fs.h | 19 +- fs/btrfs/inode-item.c | 8 +- fs/btrfs/inode.c | 140 ++-- fs/btrfs/ioctl.c | 14 +- fs/btrfs/lzo.c | 80 +-- fs/btrfs/print-tree.c | 14 +- fs/btrfs/qgroup.c | 10 +- fs/btrfs/qgroup.h | 2 +- fs/btrfs/raid56.c | 810 ++++++++++++------------ fs/btrfs/raid56.h | 36 +- fs/btrfs/reflink.c | 22 +- fs/btrfs/relocation.c | 16 +- fs/btrfs/scrub.c | 442 ++++++------- fs/btrfs/send.c | 36 +- fs/btrfs/subpage.c | 92 +-- fs/btrfs/subpage.h | 8 +- fs/btrfs/super.c | 10 +- fs/btrfs/sysfs.c | 4 +- fs/btrfs/tests/btrfs-tests.c | 38 +- fs/btrfs/tests/btrfs-tests.h | 18 +- fs/btrfs/tests/delayed-refs-tests.c | 4 +- fs/btrfs/tests/extent-buffer-tests.c | 8 +- fs/btrfs/tests/extent-io-tests.c | 34 +- fs/btrfs/tests/free-space-tests.c | 104 +-- fs/btrfs/tests/free-space-tree-tests.c | 28 +- fs/btrfs/tests/inode-tests.c | 266 ++++---- fs/btrfs/tests/qgroup-tests.c | 12 +- fs/btrfs/tests/raid-stripe-tree-tests.c | 8 +- fs/btrfs/tree-checker.c | 100 +-- fs/btrfs/tree-log.c | 6 +- fs/btrfs/volumes.c | 26 +- fs/btrfs/zlib.c | 2 +- fs/btrfs/zoned.c | 6 +- fs/btrfs/zstd.c | 6 +- include/uapi/linux/btrfs.h | 21 +- include/uapi/linux/btrfs_tree.h | 13 +- 59 files changed, 1590 insertions(+), 1569 deletions(-)