On Sun, Jun 18, 2017 at 1:57 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> On Fri, Jun 16, 2017 at 04:51:19PM +0200, Andreas Gruenbacher wrote:
>> Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
>> ---
>> 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
