| Message ID | cover.1663167823.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: initial ctree.h cleanups, simple stuff | expand |
On 2022/9/14 23:06, Josef Bacik wrote: > Hello, > > This is the first part in what will probably be very many parts in my work to > cleanup ctree.h. These are the smaller changes, mostly removing code that's not > used anymore, moving some stuff that's local to C files that don't need to be in > the header at all, and moving the rest of the on-disk definition stuff to > btrfs_tree.h. There's a lot of patche here, but they're all relatively small, > the largest being the patch to move the on-disk definitions to btrfs_tree.h, > which is not very large compared to patches in the next several series. This > has been built and tested, it's relatively low risk. Thanks, Looks good overall to me. Just 3 small points of concern: - About btrfs_tree.h usage. Really hope David can make it clear that, that UAPI header is for ALL on-disk format definition. I'm not buying the old reason that the UAPI header is only for tree search ioctl, and really want to move the whole super block definition into UAPI header. (And I also think all upstream fses should have a concentrated UAPI header too, just to make new comers easier to hack) - Some tiny inline functions got unexported May not be a big thing though, just want to make sure we have no other choice but make them uninlined. - Extra error tags in init_btrfs_fs() Just like open_ctree(), it's when but not whether to have wrong jump. Thus a new series to make those cachep related exit function conditional. (aka, we can call them unconditionally at error path) Thanks, Qu > > Josef > > Josef Bacik (17): > btrfs: remove set/clear_pending_info helpers > btrfs: remove BTRFS_TOTAL_BYTES_PINNED_BATCH > btrfs: remove BTRFS_IOPRIO_READA > btrfs: move btrfs on disk definitions out of ctree.h > btrfs: move btrfs_get_block_group helper out of disk-io.h > btrfs: move maximum limits to btrfs_tree.h > btrfs: move BTRFS_MAX_MIRRORS into scrub.c > btrfs: move discard stat defs to free-space-cache.h > btrfs: move btrfs_chunk_item_size out of ctree.h > btrfs: move btrfs_should_fragment_free_space into block-group.c > btrfs: move flush related definitions to space-info.h > btrfs: move btrfs_print_data_csum_error into inode.c > btrfs: move trans_handle_cachep out of ctree.h > btrfs: move btrfs_path_cachep out of ctree.h > btrfs: move free space cachep's out of ctree.h > btrfs: move btrfs_next_old_item into ctree.c > btrfs: move the btrfs_verity_descriptor_item defs up in ctree.h > > fs/btrfs/block-group.c | 12 + > fs/btrfs/block-group.h | 11 +- > fs/btrfs/btrfs_inode.h | 26 --- > fs/btrfs/ctree.c | 26 +++ > fs/btrfs/ctree.h | 388 ++------------------------------ > fs/btrfs/delayed-inode.c | 1 + > fs/btrfs/disk-io.c | 7 + > fs/btrfs/disk-io.h | 8 +- > fs/btrfs/free-space-cache.c | 28 +++ > fs/btrfs/free-space-cache.h | 11 + > fs/btrfs/inode-item.c | 1 + > fs/btrfs/inode.c | 57 ++--- > fs/btrfs/props.c | 1 + > fs/btrfs/relocation.c | 1 + > fs/btrfs/scrub.c | 11 + > fs/btrfs/space-info.h | 59 +++++ > fs/btrfs/super.c | 24 +- > fs/btrfs/transaction.c | 17 ++ > fs/btrfs/transaction.h | 2 + > fs/btrfs/volumes.c | 7 + > fs/btrfs/volumes.h | 2 +- > include/uapi/linux/btrfs_tree.h | 226 +++++++++++++++++++ > 22 files changed, 476 insertions(+), 450 deletions(-) >
On Thu, Sep 15, 2022 at 05:47:59PM +0800, Qu Wenruo wrote: > > > On 2022/9/14 23:06, Josef Bacik wrote: > > Hello, > > > > This is the first part in what will probably be very many parts in my work to > > cleanup ctree.h. These are the smaller changes, mostly removing code that's not > > used anymore, moving some stuff that's local to C files that don't need to be in > > the header at all, and moving the rest of the on-disk definition stuff to > > btrfs_tree.h. There's a lot of patche here, but they're all relatively small, > > the largest being the patch to move the on-disk definitions to btrfs_tree.h, > > which is not very large compared to patches in the next several series. This > > has been built and tested, it's relatively low risk. Thanks, > > Looks good overall to me. > > Just 3 small points of concern: > > - About btrfs_tree.h usage. > > Really hope David can make it clear that, that UAPI header is for ALL > on-disk format definition. > > I'm not buying the old reason that the UAPI header is only for tree > search ioctl, and really want to move the whole super block definition > into UAPI header. I have merged the patch but am expecting problems once people start using the headers. I'll be addressed case by case. > (And I also think all upstream fses should have a concentrated UAPI > header too, just to make new comers easier to hack) > > - Some tiny inline functions got unexported > > May not be a big thing though, just want to make sure we have no other > choice but make them uninlined. This depends, trivial functions or where it's on a hot path it's better to keep inline. > > - Extra error tags in init_btrfs_fs() ^^^^ labels > Just like open_ctree(), it's when but not whether to have wrong jump. > Thus a new series to make those cachep related exit function > conditional. (aka, we can call them unconditionally at error path) You've sent the series that converts it to the array and we're going to it that way. Please refresh the series as there were some minor changes to the init/exit functions and resend. Thanks.