Re: [PATCH 5/8] xfs: implement iomap based buffered write path

[Brian Foster <bfoster@redhat.com>]

On Mon, May 02, 2016 at 08:25:23PM +0200, Christoph Hellwig wrote:
> On Thu, Apr 14, 2016 at 08:58:14AM -0400, Brian Foster wrote:
> > > +static int
> > > +xfs_file_iomap_end_delalloc(
> > > +	struct xfs_inode	*ip,
> > > +	loff_t			offset,
> > > +	loff_t			length,
> > > +	ssize_t			written)
> > > +{
> > > +	struct xfs_mount	*mp = ip->i_mount;
> > > +	xfs_fileoff_t		start_fsb;
> > > +	xfs_fileoff_t		end_fsb;
> > > +	int			error = 0;
> > > +
> > > +	start_fsb = XFS_B_TO_FSB(mp, offset + written);
> > > +	end_fsb = XFS_B_TO_FSB(mp, offset + length - written);
> > > +
> > 
> > Just skimming over this series... but shouldn't this be offset + length?
> > Why walk back from the end of the allocated range?
> 
> Because the interface from the core iomap code need to pass the
> length of the actually mapped range, and the amount of bytes successfully
> written into it to the filesystem, as other filesystems will require
> this for their locking.  We need to convert it back at some point,
> and it seems more logical here than in the caller.
> 

I'm not asking about the interface... or at least I'm not following your
point. I'm just suggesting that the calculation of end_fsb is wrong.
E.g., if the intent is to punch out the range that was allocated but not
written to, shouldn't the range to punch be [offset + written, offset +
length]?

Brian

> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs