On Mon, Apr 15, 2024 at 08:58:25AM -0700, Darrick J. Wong wrote:
> On Mon, Apr 15, 2024 at 01:10:54AM -0700, Luis Chamberlain wrote:
> > +round_up_to_page_boundary()
> > +{
> > + local n=$1
> > + local page_size=$(_get_page_size)
> > +
> > + echo $(( (n + page_size - 1) & ~(page_size - 1) ))
>
> Does iomap put a large folio into the pagecache that crosses EOF
When minorder is used care had to be taken to ensure the EOF is properly
respected. Refer to the patch titled "filemap: cap PTE range to be
created to allowed zero fill in folio_map_range()", with that, we fix
that case to respect the EOF. But since minorder is used the folio is
there.
> > +mread()
> > +{
> > + local file=$1
> > + local map_len=$2
> > + local offset=$3
> > + local length=$4
> > +
> > + # Some callers expect xfs_io to crash with SIGBUS due to the mread,
> > + # causing the shell to print "Bus error" to stderr. To allow this
> > + # message to be redirected, execute xfs_io in a new shell instance.
> > + # However, for this to work reliably, we also need to prevent the new
> > + # shell instance from optimizing out the fork and directly exec'ing
> > + # xfs_io. The easiest way to do that is to append 'true' to the
> > + # commands, so that xfs_io is no longer the last command the shell sees.
> > + bash -c "trap '' SIGBUS; $XFS_IO_PROG -r $file \
> > + -c 'mmap -r 0 $map_len' \
> > + -c 'mread $offset $length'; true"
>
> Please hoist the mread() function with generic/574 to common; your copy
> is out of date with the original.
Sure thing! Shall we add _mwrite to common/rc too while at it or do we
wait to get a user for that?
> > + # 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 does "other data" mean?
That depends on the filesystem implementation, it just means we don't
provide a consistent behaviour or enforce a strategy for all filesystems.
> > + # object data can vary we just verify the filesize does not change.
> > + # This is not true for tmpfs.
>
> Er... is this file size change a bug?
There is no filesize bug, the comment about tmpfs always ensuring seeing
the actual data since, well, there its kind of write-through. Since we
share the same filemap_map_pages() I'd expect the rest should behave the
same with tmpfs, but since I didn't test that the test skips it for now.
We'll test it, with all the patch "filemap: cap PTE range to be
created to allowed zero fill in folio_map_range()" on tmpfs, and see if
we can just enable this test there too. Might as well as we're driving
by and sprinkling large folios there too.
Luis
