On Tue, Feb 5, 2019 at 9:42 AM Amir Goldstein <amir73il@xxxxxxxxx> 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.
