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