Re: [PATCH 6/7] xfs: Switch to iomap for lseek SEEK_HOLE / SEEK_DATA

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux