ltp icon indicating copy to clipboard operation
ltp copied to clipboard

Fix msync04 race

Open pevik opened this issue 1 year ago • 4 comments

msync04 suffers races. According to Jan Kara nothing guarantees that the page is not written out before get_dirty_page() manages to read the page state, test should be reworked to verify the page contents is on disk when it finds dirty bit isn't set anymore:

What would be a more sensible test is that msync is indeed persisting the data. So something like: mmap file, write to mmap, msync, abort fs, mount fs again, check the data is there. We do have framework for stuff like this in fstests (but we don't test msync AFAIK, only fsync), not sure if LTP has something for this as well [1]).

=> We should find out how fstests do filesystem abort. And also evaluate if it'd make more sense to add msync support to fstests.

[1] https://lore.kernel.org/ltp/[email protected]/

pevik avatar May 20 '24 10:05 pevik

https://patchwork.ozlabs.org/project/ltp/patch/[email protected]/

coolgw avatar May 27 '24 02:05 coolgw

New version of patch needed, updating.

coolgw avatar May 28 '24 08:05 coolgw

We merge improvement (hopefully) in 66517b89141fc455ed807f3b95e5260dcf9fb90f, but there are still some TODO from Jan Kara (https://lore.kernel.org/ltp/20240528095030.valplwc5t3m3betn@quack3/):

  • Use at least direct IO to verify the data is stored on disk now (maybe I was wrong and 66517b89141fc455ed807f3b95e5260dcf9fb90f just turns test into false positive)
  • Or add support to LTP to abort fs and remount it
  • Also support for msync() could be added to fstests (https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/) to where is infrastructure for abort fs and remount it, but they don't test msync().

@metan-ucw FYI

pevik avatar May 28 '24 12:05 pevik

At least verification for storing data on the disc was added in 21be0b196e7a79b18a5094d0083307f77654c660.

@metan-ucw Do we want to have this open for looking into abort fs or we are done? I'm not sure if anybody spents time to implement it.

pevik avatar Jun 17 '24 09:06 pevik