On Fri, Apr 05, 2024 at 01:05:01PM -0600, Andreas Dilger wrote: > On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx> wrote: > > > > Update the signature of fiemap_fill_next_extent() to allow passing a > > physical length. Update all callers to pass a 0 physical length -- since > > none set the EXTENT_HAS_PHYS_LEN flag, this value doesn't matter. > > Patch-structure-wise, it doesn't make sense to me to change all of the callers > to pass "0" as the argument to this function, and then submit a whole series > of per-filesystem patches that sets only FIEMAP_EXTENT_HAS_PHYS_LEN (but also > passes phys_len = 0, which is wrong AFAICS). > > A cleaner approach would be to rename the existing fiemap_fill_next_extent() > to fiemap_fill_next_extent_phys() that takes phys_len as an argument, and then > add a simple wrapper until all of the filesystems are updated: > > #define fiemap_fill_next_extent(info, logical, phys, log_len, flags, dev) \ > fiemap_fill_next_extent_phys(info, logical, phys, log_len, 0, flags, dev) > > Then the per-filesystem patches would involve changing over the callers to > use fiemap_fill_next_extent_phys() and setting FIEMAP_EXTENT_HAS_PHYS_LEN. Cleaner still would be to just have the callers initiaize and pass a struct fiemap_extent instead of passing around a whole bunch of integer parameters. You get more explicit naming, better typesafety - functions with a bunch of integer parametrs are not great from a type safety POV - and you can add and pass fields to fiemap_extent without having to update callers that aren't using it.
