Re: [PATCH 6/9] xfs: xfs_bmap_punch_delalloc_range() should take a byte range

Dave Chinner
2022-11-16T00:57:29Z

On Tue, Nov 15, 2022 at 03:48:33PM -0800, Darrick J. Wong wrote:
> On Tue, Nov 15, 2022 at 12:44:46AM -0800, Christoph Hellwig wrote:
> > On Tue, Nov 15, 2022 at 12:30:40PM +1100, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > >
> > > All the callers of xfs_bmap_punch_delalloc_range() jump through
> > > hoops to convert a byte range to filesystem blocks before calling
> > > xfs_bmap_punch_delalloc_range(). Instead, pass the byte range to
> > > xfs_bmap_punch_delalloc_range() and have it do the conversion to
> > > filesystem blocks internally.
> >
> > Ok, so we do this here. Why can't we just do this earlier and
> > avoid the strange inbetween stage with a wrapper?
>
> Probably to avoid rewriting the previous patch with this units change,
> is my guess. Dave, do you want to merge with this patch 4? I'm
> satisfied enough with patches 4 and 6 that I'd rather get this out to
> for-next for further testing than play more patch golf.

The fact that Christoph NAK'd exporting mapping_seek_hole_data()
this time around is just Plain Fucking Annoying. He didn't say
anything in the last thread about it after I explained why I used
it, so I had assumed it was all OK.

I've already been playing patch golf all morning now to rearrange
all the deck chairs to avoid exporting mapping_seek_hole_data().
Instead we now have an exported iomap function that wraps
mapping_seek_hole_data, and the wrapper function I added in patch 4
is now the callback function that is passed 3 layers deep into the
iomap code.

Hence the xfs_buffered_write_delalloc_punch() function needs to
remain - we can't elide it entire like this patch does - because now
we need a callback function that we can provide to the iomap code so
we avoid coupling internal XFS implementation functions to external
VFS APIs.

-Dave.
--
Dave Chinner
david@fromorbit.com