| Message ID | 1497624680-16685-7-git-send-email-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 16, 2017 at 04:51:19PM +0200, Andreas Gruenbacher wrote: > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > --- > fs/xfs/xfs_file.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index 5fb5a09..b36dcd7 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -1283,28 +1283,15 @@ xfs_seek_hole_data( > struct xfs_inode *ip = XFS_I(inode); > struct xfs_mount *mp = ip->i_mount; > uint lock; > - loff_t offset, end; > - int error = 0; > + loff_t offset; > > if (XFS_FORCED_SHUTDOWN(mp)) > return -EIO; > > lock = xfs_ilock_data_map_shared(ip); > - > - end = i_size_read(inode); > - offset = __xfs_seek_hole_data(inode, start, end, whence); > - if (offset < 0) { > - error = offset; > - goto out_unlock; > - } > - > - offset = vfs_setpos(file, offset, inode->i_sb->s_maxbytes); > - > -out_unlock: > + offset = iomap_seek_hole_data(file, start, whence, &xfs_iomap_ops); > xfs_iunlock(ip, lock); > > - if (error) > - return error; > return offset; > } Doesn't this change the way data in unwritten extents is reported? i.e. we current report unwritten extents as holes if there is no data pending writeback over the range in the page cache, and as data if there is data in the page cache. I don't see this page cache lookup for unwritten extents implemented in iomap_seek_hole_data() - it only reports extents mapped as holes as holes. Hence this will now always report unwritten extents as data . This strikes me as a regression as we currently report them as a hole: $ xfs_io -f -c "truncate 1m" -c "falloc 0 1m" -c "seek -a -r 0" foo Whence Result HOLE 0 $ I'm pretty sure that ext4 has the same behaviour when it comes to dirty page cache pages over unwritten extents .. Cheers, Dave.
On Sun, Jun 18, 2017 at 1:57 AM, Dave Chinner <david@fromorbit.com> wrote: > On Fri, Jun 16, 2017 at 04:51:19PM +0200, Andreas Gruenbacher wrote: >> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> >> --- >> fs/xfs/xfs_file.c | 17 ++--------------- >> 1 file changed, 2 insertions(+), 15 deletions(-) >> >> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c >> index 5fb5a09..b36dcd7 100644 >> --- a/fs/xfs/xfs_file.c >> +++ b/fs/xfs/xfs_file.c >> @@ -1283,28 +1283,15 @@ xfs_seek_hole_data( >> struct xfs_inode *ip = XFS_I(inode); >> struct xfs_mount *mp = ip->i_mount; >> uint lock; >> - loff_t offset, end; >> - int error = 0; >> + loff_t offset; >> >> if (XFS_FORCED_SHUTDOWN(mp)) >> return -EIO; >> >> lock = xfs_ilock_data_map_shared(ip); >> - >> - end = i_size_read(inode); >> - offset = __xfs_seek_hole_data(inode, start, end, whence); >> - if (offset < 0) { >> - error = offset; >> - goto out_unlock; >> - } >> - >> - offset = vfs_setpos(file, offset, inode->i_sb->s_maxbytes); >> - >> -out_unlock: >> + offset = iomap_seek_hole_data(file, start, whence, &xfs_iomap_ops); >> xfs_iunlock(ip, lock); >> >> - if (error) >> - return error; >> return offset; >> } > > Doesn't this change the way data in unwritten extents is reported? > i.e. we current report unwritten extents as holes if there is no > data pending writeback over the range in the page cache, and as > data if there is data in the page cache. > > I don't see this page cache lookup for unwritten extents implemented > in iomap_seek_hole_data() - it only reports extents mapped as holes > as holes. Hence this will now always report unwritten extents as > data . This strikes me as a regression as we currently report them > as a hole: > > $ xfs_io -f -c "truncate 1m" -c "falloc 0 1m" -c "seek -a -r 0" foo > Whence Result > HOLE 0 > $ Indeed, thanks for pointing this out. > I'm pretty sure that ext4 has the same behaviour when it comes to > dirty page cache pages over unwritten extents. That's correct as well. The code in xfs and ext4 looks similar enough that we can implement this generically in the iomap_seek_hole_data helper, I believe. Thanks, Andreas
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 5fb5a09..b36dcd7 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -1283,28 +1283,15 @@ xfs_seek_hole_data( struct xfs_inode *ip = XFS_I(inode); struct xfs_mount *mp = ip->i_mount; uint lock; - loff_t offset, end; - int error = 0; + loff_t offset; if (XFS_FORCED_SHUTDOWN(mp)) return -EIO; lock = xfs_ilock_data_map_shared(ip); - - end = i_size_read(inode); - offset = __xfs_seek_hole_data(inode, start, end, whence); - if (offset < 0) { - error = offset; - goto out_unlock; - } - - offset = vfs_setpos(file, offset, inode->i_sb->s_maxbytes); - -out_unlock: + offset = iomap_seek_hole_data(file, start, whence, &xfs_iomap_ops); xfs_iunlock(ip, lock); - if (error) - return error; return offset; }
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/xfs/xfs_file.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)