| Message ID | 20210826164054.14993-1-mpdesouza@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: Create macro to iterate slots | expand |
On Thu, 2021-08-26 at 13:40 -0300, Marcos Paulo de Souza wrote: > There is a common pattern when search for a key in btrfs: > > * Call btrfs_search_slot > * Endless loop > * If the found slot is bigger than the current items in the > leaf, check the > next one > * If still not found in the next leaf, return 1 > * Do something with the code > * Increment current slot, and continue > > This pattern can be improved by creating an iterator macro, similar > to > those for_each_X already existing in the linux kernel. using this > approach means to reduce significantly boilerplate code, along making > it > easier to newcomers to understand how to code works. > > This patchset survived a complete fstest run. My bad, I only added v2 to the cover-letter, but the only change from v1 is that now the xattr changes are in a separate patch. > > Changes from v1: > * Separate xattr changes from the macro introducing code (Johannes) > > Changes from RFC: > * Add documentation to btrfs_for_each_slot macro and > btrfs_valid_slot function > (David) > * Add documentation about the ret variable used as a macro argument > (David) > * Match function argument from prototype and implementation (David) > * Changed ({ }) block to only () in btrfs_for_each_slot macro > (David) > * Add more patches to show the code being reduced by using this > approach > (Nikolay) > > Marcos Paulo de Souza (8): > fs: btrfs: Introduce btrfs_for_each_slot > btrfs: block-group: use btrfs_for_each_slot in > find_first_block_group > btrfs: dev-replace: Use btrfs_for_each_slot in > mark_block_group_to_copy > btrfs: dir-item: use btrfs_for_each_slot in > btrfs_search_dir_index_item > btrfs: inode: use btrfs_for_each_slot in btrfs_read_readdir > btrfs: send: Use btrfs_for_each_slot macro > btrfs: volumes: use btrfs_for_each_slot in btrfs_read_chunk_tree > btrfs: xattr: Use btrfs_for_each_slot macro in btrfs_listxattr > > fs/btrfs/block-group.c | 33 +----- > fs/btrfs/ctree.c | 28 ++++++ > fs/btrfs/ctree.h | 25 +++++ > fs/btrfs/dev-replace.c | 51 ++-------- > fs/btrfs/dir-item.c | 27 +---- > fs/btrfs/inode.c | 46 ++++----- > fs/btrfs/send.c | 222 +++++++++++-------------------------- > ---- > fs/btrfs/volumes.c | 23 ++--- > fs/btrfs/xattr.c | 40 +++----- > 9 files changed, 169 insertions(+), 326 deletions(-) >
On Thu, Aug 26, 2021 at 01:40:46PM -0300, Marcos Paulo de Souza wrote: > Marcos Paulo de Souza (8): > fs: btrfs: Introduce btrfs_for_each_slot > btrfs: block-group: use btrfs_for_each_slot in find_first_block_group > btrfs: dev-replace: Use btrfs_for_each_slot in > mark_block_group_to_copy > btrfs: dir-item: use btrfs_for_each_slot in > btrfs_search_dir_index_item > btrfs: inode: use btrfs_for_each_slot in btrfs_read_readdir > btrfs: send: Use btrfs_for_each_slot macro > btrfs: volumes: use btrfs_for_each_slot in btrfs_read_chunk_tree > btrfs: xattr: Use btrfs_for_each_slot macro in btrfs_listxattr There are few comments regarding patch organization. Please: - drop the file prefixes from the subject (like "inode:", "send:") - change "fs: btrfs:" to just "btrfs:" in the first patch - unify the subjects that switch to the iterator to be "btrfs: use btrfs_for_each_slot in FUNCTION" - do one patch per function, even if there are several in one file
There is a common pattern when search for a key in btrfs: * Call btrfs_search_slot * Endless loop * If the found slot is bigger than the current items in the leaf, check the next one * If still not found in the next leaf, return 1 * Do something with the code * Increment current slot, and continue This pattern can be improved by creating an iterator macro, similar to those for_each_X already existing in the linux kernel. using this approach means to reduce significantly boilerplate code, along making it easier to newcomers to understand how to code works. This patchset survived a complete fstest run. Changes from v1: * Separate xattr changes from the macro introducing code (Johannes) Changes from RFC: * Add documentation to btrfs_for_each_slot macro and btrfs_valid_slot function (David) * Add documentation about the ret variable used as a macro argument (David) * Match function argument from prototype and implementation (David) * Changed ({ }) block to only () in btrfs_for_each_slot macro (David) * Add more patches to show the code being reduced by using this approach (Nikolay) Marcos Paulo de Souza (8): fs: btrfs: Introduce btrfs_for_each_slot btrfs: block-group: use btrfs_for_each_slot in find_first_block_group btrfs: dev-replace: Use btrfs_for_each_slot in mark_block_group_to_copy btrfs: dir-item: use btrfs_for_each_slot in btrfs_search_dir_index_item btrfs: inode: use btrfs_for_each_slot in btrfs_read_readdir btrfs: send: Use btrfs_for_each_slot macro btrfs: volumes: use btrfs_for_each_slot in btrfs_read_chunk_tree btrfs: xattr: Use btrfs_for_each_slot macro in btrfs_listxattr fs/btrfs/block-group.c | 33 +----- fs/btrfs/ctree.c | 28 ++++++ fs/btrfs/ctree.h | 25 +++++ fs/btrfs/dev-replace.c | 51 ++-------- fs/btrfs/dir-item.c | 27 +---- fs/btrfs/inode.c | 46 ++++----- fs/btrfs/send.c | 222 +++++++++++------------------------------ fs/btrfs/volumes.c | 23 ++--- fs/btrfs/xattr.c | 40 +++----- 9 files changed, 169 insertions(+), 326 deletions(-)