ltp
ltp copied to clipboard
Fix msync04 race
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]/
https://patchwork.ozlabs.org/project/ltp/patch/[email protected]/
New version of patch needed, updating.
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
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.