Re: [RFC][PATCH v2 0/5] Experiments with overlayfs filemap

Amir Goldstein
2019-02-05T13:03:18Z

On Tue, Feb 5, 2019 at 9:42 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> > > >
> > > > EIO on clone/dedupe:
> > > > generic/303 generic/304
> > > >
> > > > Wrong st_blocks/fiemap:
> > > > generic/353 generic/392 generic/422
> > >
> > > Hmm, generic/353 passes for me...
> > >
> >
> > So for me:
> > running generic/353 after generic/303 it fails
> > and running it again it passes.
> >
>
> The failed line in 303,304 is:
>
> xfs_io -c "pread -v -q 9223372036854775806 1" /mnt/test/foo
> pread: Input/output error
>
> And failure seems not related to clone/dedupe at all.
> Easily reproduced on any file (i.e. touch /mnt/test/foo),
> so its probably just an arithmetic len/offset bug.
>

Yes, pos+PAGE_SIZE < 0 (overrun).

Pushed a fix to truncate upper read to i_size.
It's more general although maybe non needed
for the common case, where pos+PAGE_SIZE > 0.

Interesting side effect. Now generic/353 fails always (or more often).
It appears as though a 64K reflinked file is broken to 2 extents
while the test expects it to have a single extent.

I am not sure why the test makes this expectation, maybe because
it was very probable for buffered write and made it easier to write the test.
The fact is that if we issue buffered write to upper file, the test passes
as expected, but when using direct IO to upper file the test fails.

I guess that direct IO results in different block reservation than
buffered IO in xfs. So it appears that converting buffered IO to direct IO
on upper file may have some implications.

Its quite likely that this specific issue will go away with ->writepages(),
so let's wait and see if that is an issue.
The question is whether we want to mess with hidden filesystem
implementation details or we would want to provide a knob for the
user to choose between direct and buffered IO to upper.

Double page caches can be mitigated by invalidating upper page
cache after writeback. In any case, we need to invalidate upper
page cache after copy up and in other occasions (like when upper
fs has no direct IO support), so the code for using buffered IO to
upper and cleaning upper page cache needs to be implemented anyway.

Thanks,
Amir.