xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 5/8] xfs: implement iomap based buffered write path
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Tue, 3 May 2016 11:02:19 -0400
Cc: rpeterso@xxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160502182523.GB7077@xxxxxx>
References: <1460494382-14547-1-git-send-email-hch@xxxxxx> <1460494382-14547-6-git-send-email-hch@xxxxxx> <20160414125814.GE20696@xxxxxxxxxxxxxxx> <20160502182523.GB7077@xxxxxx>
User-agent: Mutt/1.6.0 (2016-04-01)
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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>