On Wed, Oct 23, 2024 at 4:24 PM Dominique Martinet <asmadeus@xxxxxxxxxxxxx> wrote: > > Adding David/Willy to recpients as I'm not 100% up to date on folios > > Andrii Nakryiko wrote on Wed, Oct 23, 2024 at 09:56:06AM -0700: > > > The following changes since commit 98f7e32f20d28ec452afb208f9cffc08448a2652: > > > > > > Linux 6.11 (2024-09-15 16:57:56 +0200) > > > > > > are available in the Git repository at: > > > > > > https://github.com/martinetd/linux tags/9p-for-6.12-rc4 > > > > > > for you to fetch changes up to 79efebae4afc2221fa814c3cae001bede66ab259: > > > > > > 9p: Avoid creating multiple slab caches with the same name (2024-09-23 05:51:27 +0900) > > > > > > ---------------------------------------------------------------- > > > Mashed-up update that I sat on too long: > > > > > > - fix for multiple slabs created with the same name > > > - enable multipage folios > > > - theorical fix to also look for opened fids by inode if none > > > was found by dentry > > > > > > ---------------------------------------------------------------- > > > David Howells (1): > > > 9p: Enable multipage folios > > > > Are there any known implications of this change on madvise()'s MADV_PAGEOUT > > behavior? After most recent pull from Linus's tree, one of BPF selftests > > started failing. Bisection points to: > > > > 9197b73fd7bb ("Merge tag '9p-for-6.12-rc4' of https://github.com/martinetd/linux";) > > > > ... which is just an empty merge commit. So the "9p: Enable multipage folios" > > by itself doesn't cause any regression, but when merged with the rest of the > > code it does. I confirmed by reverting > > 1325e4a91a40 ("9p: Enable multipage folios"), after which the test in question > > is succeeding again. > > (looks like 3c217a182018 ("selftests/bpf: add build ID tests") wasn't in > yet on the 9p multipage folios commit) > > > The test in question itself is a bit involved, but what it ultimately tries to > > do is to ensure that part of ELF file containing build ID is paged out to cause > > BPF helper to fail to retrieve said build ID (due to non-faulable context). > > > > For that, we use the following sequence in target binary and process: > > > > madvise(addr, page_sz, MADV_POPULATE_READ); > > madvise(addr, page_sz, MADV_PAGEOUT); > > > > First making sure page is paged in, then paged out. We make sure that build ID > > is memory mapped in a separate segment with its own single-page memory mapping. > > No changes or regressions there. No huge pages seem to be involved. > > That's probably obvious but I guess the selftest runs the binary > directly from a 9p mount? Yep, should have pointed that out explicitly. > > > It used to work reliably, now it doesn't work. Any clue why or what should we > > do differently to make sure that memory page with build ID information is not > > paged in (reliably)? > > Unless David/Willy has a solution immediately I'd say let's take the time to > sort this out and revert that commit for now -- I'll send a revert patch > immediately and submit it to Linus on Saturday. > > Conceptually I guess something is broken with MADV_PAGEOUT on >1 page > folio, perhaps it's only evicting folios if the whole folio is in range > but it should evict any folio that touches the range or something? Could be, yeah. It's not necessarily a bug of 9P itself, but it would be nice to have some way to page out memory. Maybe we need some extra flags or a new MADV_PAGEOUT_OVERLAPPING command for madvise(), or something along those lines? > > Sorry I don't have time to dig further here, hopefully that's not too > difficult to handle and we can try again in rc1 proper of another cycle, > I shouldn't have sent that this late. > No worries, thanks for a quick reply! > > (leaving full text below for new recipients) > > Thanks! > > > > P.S. The target binary and madvise() manipulations are at: > > > > tools/testing/selftests/bpf/uprobe_multi.c, see trigger_uprobe() > > The test itself in BPF selftest is at: > > > > tools/testing/selftests/bpf/prog_tests/build_id.c, see subtest_nofault(), > > build_id_resident is false in this case. > > > > > > > > Dominique Martinet (1): > > > 9p: v9fs_fid_find: also lookup by inode if not found dentry > > > > > > Pedro Falcato (1): > > > 9p: Avoid creating multiple slab caches with the same name > > > > > > fs/9p/fid.c | 5 ++--- > > > fs/9p/vfs_inode.c | 1 + > > > net/9p/client.c | 10 +++++++++- > > > 3 files changed, 12 insertions(+), 4 deletions(-) > > > > > > > Thanks, > -- > Dominique Martinet | Asmadeus
