On Mon, May 30, 2022 at 4:29 PM Zorro Lang <zlang@xxxxxxxxxx> wrote: > > On Mon, May 30, 2022 at 02:29:05PM +0300, Amir Goldstein wrote: > > For this test to run on overlayfs we open a different file to perform > > shutdown+fsync while keeping the writeback target file open. > > > > We should probably perform fsync on the writeback target file, but > > the bug is reproduced on xfs and overlayfs+xfs also as is. > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > --- > > > > Zorro, > > > > I tested that this test passes for both xfs and overlayfs+xfs on v5.18 > > and tested that both configs fail with the same warning on v5.10.109. > > > > I tried changing fsync to syncfs for the test to be more correct in the > > overlayfs case, but then golden output of xfs and overlayfs+xfs differ > > and that would need some more output filtering (or disregarding output > > completely). > > > > Since this minimal change does the job and does not change test behavior > > on xfs on any of the tested kernels, I thought it might be good enough. > > > > Thanks, > > Amir. > > > > tests/generic/623 | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tests/generic/623 b/tests/generic/623 > > index ea016d91..bb36ad25 100755 > > --- a/tests/generic/623 > > +++ b/tests/generic/623 > > @@ -24,10 +24,13 @@ _scratch_mount > > # XFS had a regression where it failed to check shutdown status in the fault > > # path. This produced an iomap warning because writeback failure clears Uptodate > > # status on the page. > > +# For this test to run on overlayfs we open a different file to perform > > +# shutdown+fsync while keeping the writeback target file open. > > file=$SCRATCH_MNT/file > > $XFS_IO_PROG -fc "pwrite 0 4k" -c fsync $file | _filter_xfs_io > > ulimit -c 0 > > -$XFS_IO_PROG -x -c "mmap 0 4k" -c "mwrite 0 4k" -c shutdown -c fsync \ > > +$XFS_IO_PROG -x -c "mmap 0 4k" -c "mwrite 0 4k" \ > > + -c "open $(_scratch_shutdown_handle)" -c shutdown -c fsync \ > > Did you try to reproduce the original bug which this test case covers? > Yes. As I wrote: "tested that both configs fail with the same warning on v5.10.109" Meaning the same bug that the test triggered before my change in v5.10 is still triggered on xfs in v5.10 and it is triggered on both xfs and overlayfs+xfs in v5.10 with my change. > According to the "man xfs_io": > > open [[ -acdfrstRTPL ] path ] > Closes the current file, and opens the file specified by path instead. The documentation is incorrect. Current file is not closed. > Although I doubt if it always real close the current file, but you open to get > a new file descriptor, later operations will base on new fd. I don't know if > it still has original testing coverage. fsync on the fs root dir is not the same as fsync on the original file. mwrite does not change because mwrite is not acted on open fd it is acted on memory mapping of mmap. I can either change fd again to first fd before doing fsync or change fsync to syncfs. Both solutions should be fine, just a bit more work on the golden output. I just wonder if we need to do anything at all as the bug is reproduced and warning triggered anyway. > > I'd like to help this case to support overlay, presuppose original fs > testing isn't changed. Or it's not worth. > > Welcome more review points from the author of this case and others. > Yes, me as well. Thanks, Amir.
