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

Brian Foster
2016-05-03T15:02:25Z

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