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.
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.
