real_inode->i_fop->llseek is not called by ovl_llseek

real_inode->i_fop->llseek is not called by ovl_llseek

[Eddie Horng]

Hello,
Seems real_inode->i_fop->llseek is not called by ovl_llseek?
It happens the lower fs implements llseek similar with ovl_llseek or lower fs
is overlayfs, the "real inode"  s_maxbytes is not passed to
generic_file_llseek_size. Generally it is not a problem, but is it
better to check if realinode has llseek and invoke it?

static loff_t ovl_llseek(struct file *file, loff_t offset, int whence)
{
struct inode *realinode = ovl_inode_real(file_inode(file));

return generic_file_llseek_size(file, offset, whence,
realinode->i_sb->s_maxbytes,
i_size_read(realinode));
}

thanks,
Eddie

Re: real_inode->i_fop->llseek is not called by ovl_llseek

[Amir Goldstein]

On Tue, Feb 26, 2019 at 8:31 AM Eddie Horng <eddiehorng.tw@gmail.com> wrote:
>
> Hello,
> Seems real_inode->i_fop->llseek is not called by ovl_llseek?
> It happens the lower fs implements llseek similar with ovl_llseek or lower fs
> is overlayfs, the "real inode"  s_maxbytes is not passed to
> generic_file_llseek_size. Generally it is not a problem, but is it
> better to check if realinode has llseek and invoke it?

Yes, it is a problem!

We have a silent regression in v4.19.
Overlayfs used to do SEEK_HOLE/SEEK_DATA correctly
and does not anymore.

Here is head of full output of xfstests seek hole sanity test:

XFS
===
root@kvm-xfstests:~# head -3 /results/xfs/results-reflink/generic/285.full
File system magic#: 0x58465342
Allocation size: 4096

Overlayfs (v4.18)
=============
root@kvm-xfstests:~# head -3 /results/overlay/results-large/generic/285.full
File system magic#: 0x794c7630
Allocation size: 4096

Overlayfs (master)
==============
root@kvm-xfstests:~# head -3 /results/overlay/results-large/generic/285.full
File system supports the default behavior.
File system does not support unwritten extents.
File system magic#: 0x794c7630

The reason that regression went unnoticed is that xfstests seek hole
sanity tests does not require SEEK_HOLE to find a punched hole.

AFAICT, there are 11 filesystems in tree that support PUNCH_HOLE:
btrfs ceph cifs ext4 f2fs fuse gfs2 hugetlbfs nfs ocfs2 xfs

Out of which only hugetlbfs does not implement SEEK_HOLE,
and this could probably be considered a bug if anyone cared enough.
so I propose that src/seek_sanity_test.c:test_basic_support()
should check for PUNCH_HOLE support and fail if filesystem is
found to "support the default behavior".

Besides that, there is no actual sanity test in xfstests (or I couldn't
find one) that verifies PUNCH_HOLE=>SEEK_HOLE.
There is an LTP test fallocate04 that checks that, but there is
no coverage to overlayfs with .all_filesystems = 1 test.
So I will add a test case to xfstest seek_sanity_test to cover this HOLE.

Thanks,
Amir.