From: Luis Chamberlain <mcgrof@kernel.org>
To: "Darrick J. Wong" <djwong@kernel.org>, hughd@google.com
Cc: patches@lists.linux.dev, fstests@vger.kernel.org,
linux-xfs@vger.kernel.org, linux-mm@kvack.org,
linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org,
ziy@nvidia.com, vbabka@suse.cz, seanjc@google.com,
willy@infradead.org, david@redhat.com, hughd@google.com,
linmiaohe@huawei.com, muchun.song@linux.dev, osalvador@suse.de,
p.raghav@samsung.com, da.gomez@samsung.com, hare@suse.de,
john.g.garry@oracle.com
Subject: Re: [PATCH 2/5] fstests: add mmap page boundary tests
Date: Tue, 11 Jun 2024 13:29:51 -0700 [thread overview]
Message-ID: <Zmizv_g728OwNFrg@bombadil.infradead.org> (raw)
In-Reply-To: <20240611184603.GA52987@frogsfrogsfrogs>
On Tue, Jun 11, 2024 at 11:46:03AM -0700, Darrick J. Wong wrote:
> 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.
Sure, but I suspect the real issue is if it does something like leak
data which it should not. The test as-is does test to ensure the data
is zeroed.
If we want to add a test to close the mmap and ensure the data beyond
the file content up to PAGE_SIZE is still zeroed out, it's easy to do,
it was just that it seems that *could* end up with different results
depending on the filesystem.
> That said -- let's say you have a 33k file, and a space mapping for
> 0-63k
What block size filesystem in this example? If the lengh is 33k, whether
or not it was truncated does not matter, the file size is what matters.
The block size is what we use for the minimum order folio, and sincee we
start at offset 0, a 33k sized file on a 64k block size filesystem will
get a 64k folio. On a 32k block size filesystem, it will get two 32k
foios.
> (e.g. it was preallocated).
Do you mean sparse or what? Because if its a sparse file it will still
change the size of the file, so just wanted to check.
> Can the pagecache grab (say) a 64k folio for the EOF part of the pagecache?
It depends on the block size of the filesystem. If 4k, we'd go up to
36k, and 33k-46k would be zereod.
With min order, we'd have a folio of 8k, 32k, or 64k. For 8k we'd have
5 folios of 8k size each, the last one have only 1k of data, and 3k
zeroed out. No PTEs would be assigned for that folio beyond 36k boundary
and so we'd SIGBUS on access beyond it. We test for this in this test.
> And can you mmap that whole region?
No, we test for this too here. You can only mmap up to the aligned
PAGE_SIZE of the file size.
> And see even more grey area mmapping?
No, we limit up to PAGE_SIZE alignement.
> Or does mmap always cut
> off the mapping at roundup(i_size_read(), PAGE_SIZE) ?
That's right, we do this, without LBS this was implied, but with LBS
we have to be explicit about using the PAGE_SIZE alignment restriction.
This test checks for all that, and checks for both integrity of the contents
and file size even if you muck with the extra fluff allowed by mmap().
> > > 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. ;)
From the mmap(2) man page: "subsequent mappings may see the modified content."
so I extended this with the implications of it using *may*.
Speaking of the man page, I see also that huge pages are addressed there
and when a huge page is used it says:
"The system automatically aligns length to be a multiple of the
underlying huge page size"
And so I believes that means we need to check for the huge page on
filemap_map_pages() and also the test and adjust it to align to the
specific huge page size if used...
Or just skip tmpfs / hugetlbfs for now...
Luis
next prev parent reply other threads:[~2024-06-11 20:29 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-11 3:01 [PATCH 0/5] fstests: add some new LBS inspired tests Luis Chamberlain
2024-06-11 3:01 ` [PATCH 1/5] common: move mread() to generic helper _mread() Luis Chamberlain
2024-06-11 14:28 ` Darrick J. Wong
2024-06-11 3:01 ` [PATCH 2/5] fstests: add mmap page boundary tests Luis Chamberlain
2024-06-11 16:48 ` Darrick J. Wong
2024-06-11 18:10 ` Luis Chamberlain
2024-06-11 18:46 ` Darrick J. Wong
2024-06-11 20:29 ` Luis Chamberlain [this message]
2024-06-12 8:06 ` Zorro Lang
2024-06-13 21:05 ` Luis Chamberlain
2024-06-11 3:02 ` [PATCH 3/5] fstests: add fsstress + compaction test Luis Chamberlain
2024-06-11 14:48 ` Darrick J. Wong
2024-06-12 8:00 ` Zorro Lang
2024-06-13 21:10 ` Luis Chamberlain
2024-06-11 3:02 ` [PATCH 4/5] _require_debugfs(): simplify and fix for debian Luis Chamberlain
2024-06-11 14:35 ` Darrick J. Wong
2024-06-12 7:51 ` Zorro Lang
2024-06-11 3:02 ` [PATCH 5/5] fstests: add stress truncation + writeback test Luis Chamberlain
2024-06-11 14:45 ` Darrick J. Wong
2024-06-11 18:15 ` Luis Chamberlain
2024-06-11 18:29 ` Darrick J. Wong
2024-06-11 18:59 ` Luis Chamberlain
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zmizv_g728OwNFrg@bombadil.infradead.org \
--to=mcgrof@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=da.gomez@samsung.com \
--cc=david@redhat.com \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=hare@suse.de \
--cc=hughd@google.com \
--cc=john.g.garry@oracle.com \
--cc=linmiaohe@huawei.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=muchun.song@linux.dev \
--cc=osalvador@suse.de \
--cc=p.raghav@samsung.com \
--cc=patches@lists.linux.dev \
--cc=seanjc@google.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
--cc=ziy@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).