Re: [PATCH 2/5] fstests: add mmap page boundary tests

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 11, 2024 at 11:10:13AM -0700, Luis Chamberlain wrote:
> 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?

I doubt there's an easy way to guarantee more than "initialized to zero,
contents may stay around in memory but will not be written to disk".
You could do asinine things like fault on every access and manually
inject zero bytes, but ... yuck.

That said -- let's say you have a 33k file, and a space mapping for
0-63k (e.g. it was preallocated).  Can the pagecache grab (say) a 64k
folio for the EOF part of the pagecache?  And can you mmap that whole
region?  And see even more grey area mmapping?  Or does mmap always cut
off the mapping at roundup(i_size_read(), PAGE_SIZE) ?

(I feel like I've asked this before, and forgotten the answer. :()

> > > +	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?

Oh, ok.  I couldn't tell if *you* had seen "other" data emerging from
the murk, or if that was merely what a spec says.  Please cite the
particular bible you were reading. ;)

> > > +	#    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!

NP.

--D

> 
>   Luis
>