On Tue, Jun 11, 2024 at 09:48:11AM -0700, Darrick J. Wong wrote:
> On Mon, Jun 10, 2024 at 08:01:59PM -0700, Luis Chamberlain wrote:
> > +# As per POSIX NOTES mmap(2) maps multiples of the system page size, but if the
> > +# data mapped is not multiples of the page size the remaining bytes are zeroed
> > +# out when mapped and modifications to that region are not written to the file.
> > +# On Linux when you write data to such partial page after the end of the
> > +# object, the data stays in the page cache even after the file is closed and
> > +# unmapped and even though the data is never written to the file itself,
> > +# subsequent mappings may see the modified content. If you go *beyond* this
>
> Does this happen (mwrite data beyond eof sticks around) with large
> folios as well?
That corner case of checking to see if it stays is not tested by this
test, but we could / should extend this test later for that. But then
the question becomes, what is right, given we are in grey area, if we
don't have any defined standard for it, it seems odd to test for it.
So the test currently only tests for correctness of what we expect for
POSIX and what we all have agreed for Linux.
Hurding everyone to follow suit for the other corner cases is something
perhaps we should do. Do we have a "strict fail" ? So that perhaps we can
later add a test case for it and so that onnce and if we get consensus
on what we do we can enable say a "strict-Linux" mode where we are
pedantic about a new world order?
> > + rm -rf "${SCRATCH_MNT:?}"/*
>
> rm -f $SCRATCH_MNT/file ?
Sure.
> > + # A couple of mmap() tests:
> > + #
> > + # We are allowed to mmap() up to the boundary of the page size of a
> > + # data object, but there a few rules to follow we must check for:
> > + #
> > + # a) zero-fill test for the data: POSIX says we should zero fill any
> > + # partial page after the end of the object. Verify zero-fill.
> > + # b) do not write this bogus data to disk: on Linux, if we write data
> > + # to a partially filled page, it will stay in the page cache even
> > + # after the file is closed and unmapped even if it never reaches the
> > + # file. Subsequent mappings *may* see the modified content, but it
> > + # also can get other data. Since the data read after the actual
>
> What other data?
Beats me, got that from the man page bible on mmap. I think its homework
for us to find out who is spewing that out, which gives a bit more value
to the idea of that strict-linux thing. How else will we find out?
> > + # object data can vary we just verify the filesize does not change.
> > + if [[ $map_len -gt $new_filelen ]]; then
> > + zero_filled_data_len=$((map_len - new_filelen))
> > + _scratch_cycle_mount
> > + expected_zero_data="00"
> > + zero_filled_data=$($XFS_IO_PROG -r $test_file \
> > + -c "mmap -r 0 $map_len" \
> > + -c "mread -v $new_filelen $zero_filled_data_len" \
> > + -c "munmap" | \
> > + filter_xfs_io_data_unique)
> > + if [[ "$zero_filled_data" != "$expected_zero_data" ]]; then
> > + echo "Expected data: $expected_zero_data"
> > + echo " Actual data: $zero_filled_data"
> > + _fail "Zero-fill expectations with mmap() not respected"
> > + fi
> > +
> > + _scratch_cycle_mount
> > + $XFS_IO_PROG $test_file \
> > + -c "mmap -w 0 $map_len" \
> > + -c "mwrite $new_filelen $zero_filled_data_len" \
> > + -c "munmap"
> > + sync
> > + csum_post="$(_md5_checksum $test_file)"
> > + if [[ "$csum_orig" != "$csum_post" ]]; then
> > + echo "Expected csum: $csum_orig"
> > + echo " Actual csum: $csum_post"
> > + _fail "mmap() write up to page boundary should not change actual file contents"
>
> Do you really want to stop the test immediately? Or keep going and see
> what other errors fall out? (i.e. s/_fail/echo/ here)
Good point. We could go on, I'll change on the next v2.
Thanks!
Luis
