| Message ID | cover.1663175597.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: split out larger chunks of ctree.h | expand |
On 2022/9/15 01:18, Josef Bacik wrote: > Hello, > > This series is based on the series > > btrfs: initial ctree.h cleanups, simple stuff > > which needs to be in place before applying these patches. > > This is likely going to have the largest patch of the series, which bulk moves > all of the struct funcs defines out of ctree.h into their own file. This isn't > really possible to do piecemeal like other changes because we're using macros > instead of functions. However the code is well organized so it allows for a > bulk copy and paste, so is straightforward. > > I've done my best with naming, but I'm open to suggestions. My general plan is > to have all fs wide definitions in fs.h, and then separate out individual things > to their own headers. > > The biggest things I've done in this series are > > 1. Move the printk helpers into their own files. > 2. Move the main state flags and core fs helpers into their own files. > 3. Moved the struct func definitions to their own files. > > This is by no means complete, this is just the first big pass, but as you can > see is already 17 patches long. Subsequent patches will move more code and do > more cleanups. Thanks, > > Josef > > Josef Bacik (16): > btrfs: move fs wide helpers out of ctree.h > btrfs: move larger compat flag helpers to their own c file > btrfs: move the printk helpers out of ctree.h > btrfs: push extra checks into __btrfs_abort_transaction > btrfs: move assert and error helpers out of btrfs-printk.h > btrfs: push printk index code into their respective helpers > btrfs: move BTRFS_FS_STATE* defs and helpers to fs.h > btrfs: move incompat and compat flag helpers to fs.c > btrfs: move mount option definitions to fs.h > btrfs: move fs_info->flags enum to fs.h > btrfs: add a BTRFS_FS_NEED_TRANS_COMMIT flag > btrfs: remove fs_info::pending_changes and related code > btrfs: move the compat/incompat flag masks to fs.h > btrfs: rename struct-funcs.c -> item-accessors.c > btrfs: move btrfs_map_token to item-accessors > btrfs: move accessor helpers into item-accessors.h Oh my god, I have "accessors.h" in my btrfs-fuse project from day 1, finally can see it in upstream btrfs. But one thing to mention is, "item" looks more leaf oriented, while I believe node accessors would also be in the same header? Thus what about just plain "accessors.h"? Thanks, Qu > > fs/btrfs/Makefile | 4 +- > fs/btrfs/backref.c | 2 + > fs/btrfs/backref.h | 1 + > fs/btrfs/block-group.c | 2 + > fs/btrfs/block-rsv.c | 2 + > fs/btrfs/btrfs-printk.h | 206 ++ > fs/btrfs/check-integrity.c | 2 + > fs/btrfs/compression.c | 1 + > fs/btrfs/ctree.c | 3 + > fs/btrfs/ctree.h | 1784 +---------------- > fs/btrfs/delalloc-space.c | 2 + > fs/btrfs/delayed-inode.c | 3 + > fs/btrfs/delayed-ref.c | 2 + > fs/btrfs/dev-replace.c | 2 + > fs/btrfs/dir-item.c | 2 + > fs/btrfs/discard.c | 1 + > fs/btrfs/disk-io.c | 8 +- > fs/btrfs/export.c | 1 + > fs/btrfs/extent-io-tree.c | 1 + > fs/btrfs/extent-tree.c | 2 + > fs/btrfs/extent_io.c | 2 + > fs/btrfs/extent_map.c | 1 + > fs/btrfs/file-item.c | 3 + > fs/btrfs/file.c | 2 + > fs/btrfs/free-space-cache.c | 3 + > fs/btrfs/free-space-tree.c | 3 + > fs/btrfs/fs.c | 108 + > fs/btrfs/fs.h | 320 +++ > fs/btrfs/inode-item.c | 2 + > fs/btrfs/inode.c | 2 + > fs/btrfs/ioctl.c | 2 + > fs/btrfs/{struct-funcs.c => item-accessors.c} | 10 + > fs/btrfs/item-accessors.h | 1104 ++++++++++ > fs/btrfs/lzo.c | 1 + > fs/btrfs/ordered-data.c | 1 + > fs/btrfs/print-tree.c | 2 + > fs/btrfs/props.c | 3 + > fs/btrfs/qgroup.c | 2 + > fs/btrfs/raid56.c | 1 + > fs/btrfs/ref-verify.c | 3 + > fs/btrfs/reflink.c | 2 + > fs/btrfs/relocation.c | 2 + > fs/btrfs/root-tree.c | 3 + > fs/btrfs/scrub.c | 2 + > fs/btrfs/send.c | 1 + > fs/btrfs/space-info.c | 2 + > fs/btrfs/subpage.c | 1 + > fs/btrfs/super.c | 51 +- > fs/btrfs/sysfs.c | 7 +- > fs/btrfs/tests/btrfs-tests.c | 1 + > fs/btrfs/tests/extent-buffer-tests.c | 1 + > fs/btrfs/tests/free-space-tree-tests.c | 1 + > fs/btrfs/tests/inode-tests.c | 1 + > fs/btrfs/tests/qgroup-tests.c | 2 + > fs/btrfs/transaction.c | 29 +- > fs/btrfs/transaction.h | 1 - > fs/btrfs/tree-checker.c | 3 + > fs/btrfs/tree-defrag.c | 1 + > fs/btrfs/tree-log.c | 2 + > fs/btrfs/tree-log.h | 1 + > fs/btrfs/tree-mod-log.c | 3 + > fs/btrfs/ulist.c | 1 + > fs/btrfs/uuid-tree.c | 4 +- > fs/btrfs/verity.c | 3 + > fs/btrfs/volumes.c | 2 + > fs/btrfs/xattr.c | 2 + > fs/btrfs/zoned.c | 2 + > fs/btrfs/zoned.h | 1 + > 68 files changed, 1915 insertions(+), 1823 deletions(-) > create mode 100644 fs/btrfs/btrfs-printk.h > create mode 100644 fs/btrfs/fs.c > create mode 100644 fs/btrfs/fs.h > rename fs/btrfs/{struct-funcs.c => item-accessors.c} (96%) > create mode 100644 fs/btrfs/item-accessors.h >
On Wed, Sep 14, 2022 at 01:18:05PM -0400, Josef Bacik wrote: > Hello, > > This series is based on the series > > btrfs: initial ctree.h cleanups, simple stuff > > which needs to be in place before applying these patches. > > This is likely going to have the largest patch of the series, which bulk moves > all of the struct funcs defines out of ctree.h into their own file. This isn't > really possible to do piecemeal like other changes because we're using macros > instead of functions. However the code is well organized so it allows for a > bulk copy and paste, so is straightforward. > > I've done my best with naming, but I'm open to suggestions. My general plan is > to have all fs wide definitions in fs.h, and then separate out individual things > to their own headers. The fs.h feels like another ctree.h but we have to start somewhere and further moving stuff from fs.h sounds like a plan. > > The biggest things I've done in this series are > > 1. Move the printk helpers into their own files. > 2. Move the main state flags and core fs helpers into their own files. > 3. Moved the struct func definitions to their own files. > > This is by no means complete, this is just the first big pass, but as you can > see is already 17 patches long. Subsequent patches will move more code and do > more cleanups. Thanks, I'll try to merge such straightforward patchsets without much delay as long as the changes are straightforward and reasonable assuming that we'll reach perfection eventually. This series can be used as reference for what is OK, with some minor fixups or renames. The name of btrfs-printk.h stands out as it's using two subsystems while I'd expect something more decriptive of what's inside like messages.h. For that a resend is not necessary, just that we agree that a rename is fine.
On Thu, Sep 15, 2022 at 05:51:12PM +0800, Qu Wenruo wrote: > On 2022/9/15 01:18, Josef Bacik wrote: > > btrfs: move accessor helpers into item-accessors.h > > Oh my god, I have "accessors.h" in my btrfs-fuse project from day 1, > finally can see it in upstream btrfs. Good for you, you don't have to repeat the same mistakes and copy the file naming scheme in a fresh project, that's the advantage. In an actively developed yet old code base it's not for free. We basically have to stop for one dev cycle and do only cleanups and renaming and this will haunt us for years when doing backports. I'm not looking forward to that but we need it. > But one thing to mention is, "item" looks more leaf oriented, while I > believe node accessors would also be in the same header? > > Thus what about just plain "accessors.h"? Yeah probably accessors.h will be the final name.
Hello, This series is based on the series btrfs: initial ctree.h cleanups, simple stuff which needs to be in place before applying these patches. This is likely going to have the largest patch of the series, which bulk moves all of the struct funcs defines out of ctree.h into their own file. This isn't really possible to do piecemeal like other changes because we're using macros instead of functions. However the code is well organized so it allows for a bulk copy and paste, so is straightforward. I've done my best with naming, but I'm open to suggestions. My general plan is to have all fs wide definitions in fs.h, and then separate out individual things to their own headers. The biggest things I've done in this series are 1. Move the printk helpers into their own files. 2. Move the main state flags and core fs helpers into their own files. 3. Moved the struct func definitions to their own files. This is by no means complete, this is just the first big pass, but as you can see is already 17 patches long. Subsequent patches will move more code and do more cleanups. Thanks, Josef Josef Bacik (16): btrfs: move fs wide helpers out of ctree.h btrfs: move larger compat flag helpers to their own c file btrfs: move the printk helpers out of ctree.h btrfs: push extra checks into __btrfs_abort_transaction btrfs: move assert and error helpers out of btrfs-printk.h btrfs: push printk index code into their respective helpers btrfs: move BTRFS_FS_STATE* defs and helpers to fs.h btrfs: move incompat and compat flag helpers to fs.c btrfs: move mount option definitions to fs.h btrfs: move fs_info->flags enum to fs.h btrfs: add a BTRFS_FS_NEED_TRANS_COMMIT flag btrfs: remove fs_info::pending_changes and related code btrfs: move the compat/incompat flag masks to fs.h btrfs: rename struct-funcs.c -> item-accessors.c btrfs: move btrfs_map_token to item-accessors btrfs: move accessor helpers into item-accessors.h fs/btrfs/Makefile | 4 +- fs/btrfs/backref.c | 2 + fs/btrfs/backref.h | 1 + fs/btrfs/block-group.c | 2 + fs/btrfs/block-rsv.c | 2 + fs/btrfs/btrfs-printk.h | 206 ++ fs/btrfs/check-integrity.c | 2 + fs/btrfs/compression.c | 1 + fs/btrfs/ctree.c | 3 + fs/btrfs/ctree.h | 1784 +---------------- fs/btrfs/delalloc-space.c | 2 + fs/btrfs/delayed-inode.c | 3 + fs/btrfs/delayed-ref.c | 2 + fs/btrfs/dev-replace.c | 2 + fs/btrfs/dir-item.c | 2 + fs/btrfs/discard.c | 1 + fs/btrfs/disk-io.c | 8 +- fs/btrfs/export.c | 1 + fs/btrfs/extent-io-tree.c | 1 + fs/btrfs/extent-tree.c | 2 + fs/btrfs/extent_io.c | 2 + fs/btrfs/extent_map.c | 1 + fs/btrfs/file-item.c | 3 + fs/btrfs/file.c | 2 + fs/btrfs/free-space-cache.c | 3 + fs/btrfs/free-space-tree.c | 3 + fs/btrfs/fs.c | 108 + fs/btrfs/fs.h | 320 +++ fs/btrfs/inode-item.c | 2 + fs/btrfs/inode.c | 2 + fs/btrfs/ioctl.c | 2 + fs/btrfs/{struct-funcs.c => item-accessors.c} | 10 + fs/btrfs/item-accessors.h | 1104 ++++++++++ fs/btrfs/lzo.c | 1 + fs/btrfs/ordered-data.c | 1 + fs/btrfs/print-tree.c | 2 + fs/btrfs/props.c | 3 + fs/btrfs/qgroup.c | 2 + fs/btrfs/raid56.c | 1 + fs/btrfs/ref-verify.c | 3 + fs/btrfs/reflink.c | 2 + fs/btrfs/relocation.c | 2 + fs/btrfs/root-tree.c | 3 + fs/btrfs/scrub.c | 2 + fs/btrfs/send.c | 1 + fs/btrfs/space-info.c | 2 + fs/btrfs/subpage.c | 1 + fs/btrfs/super.c | 51 +- fs/btrfs/sysfs.c | 7 +- fs/btrfs/tests/btrfs-tests.c | 1 + fs/btrfs/tests/extent-buffer-tests.c | 1 + fs/btrfs/tests/free-space-tree-tests.c | 1 + fs/btrfs/tests/inode-tests.c | 1 + fs/btrfs/tests/qgroup-tests.c | 2 + fs/btrfs/transaction.c | 29 +- fs/btrfs/transaction.h | 1 - fs/btrfs/tree-checker.c | 3 + fs/btrfs/tree-defrag.c | 1 + fs/btrfs/tree-log.c | 2 + fs/btrfs/tree-log.h | 1 + fs/btrfs/tree-mod-log.c | 3 + fs/btrfs/ulist.c | 1 + fs/btrfs/uuid-tree.c | 4 +- fs/btrfs/verity.c | 3 + fs/btrfs/volumes.c | 2 + fs/btrfs/xattr.c | 2 + fs/btrfs/zoned.c | 2 + fs/btrfs/zoned.h | 1 + 68 files changed, 1915 insertions(+), 1823 deletions(-) create mode 100644 fs/btrfs/btrfs-printk.h create mode 100644 fs/btrfs/fs.c create mode 100644 fs/btrfs/fs.h rename fs/btrfs/{struct-funcs.c => item-accessors.c} (96%) create mode 100644 fs/btrfs/item-accessors.h