On 2020/12/19 上午8:26, Qu Wenruo wrote:
On 2020/12/18 下午11:57, David Sterba wrote:
On Fri, Dec 18, 2020 at 01:16:59PM +0800, Qu Wenruo wrote:
This small patchset is btrfs_dec_test_*_ordered_extent() refactor during
subpage RW support development.
This is mostly to make btrfs_dev_test_* functions more human readable
and prepare it for calling btrfs_dec_test_first_ordered_extent() in
btrfs_writepage_endio_finish_ordered() where we can have one or more
ordered extents for one bvec.
Qu Wenruo (2):
btrfs: make btrfs_dio_private::bytes to be u32
btrfs: refactor btrfs_dec_test_* functions for ordered extents
The idea makes sense but the patches are IMO in wrong order and still
leave some u64/u32, eg. in btrfs_dec_test_first_ordered_pending the
io_size is still u64 while for btrfs_dec_test_ordered_pending it
switches type to u32 (as expected).
That u64 is left there and the reason is explained in the 2nd patch.
Mostly due to iomap requirement.
But I totally get your point.
After digging more, I prefer to give up the width reduction, but just
focus on the other cleanups in the patch.
The width reduction really needs more concern, especially when iomap is
involved.
Thanks,
Qu
Thanks,
Qu
The type cleanup should be done bottom-up, from the leaf functions
upwards. After that, the structure type can be safely switched.
I'm not sure what to do with __endio_write_update_ordered, it can take
u32 for bytes but internally uses u64 for ordered_bytes, that should be
u32 as well. But
7711 if (ordered_offset < offset + bytes) {
7712 ordered_bytes = offset + bytes -
ordered_offset;
7713 ordered = NULL;
7714 }
expression on line 7712 would need a temporary variable for the u64
calculation and then reassign. Maybe such conversions are inevitable so
we have clean function API and not pass u64 just because.
I like that the hole btrfs_dio_private gets removed so the cleanups are
worthwhile, but maybe not trivial.
