All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gregory Price <gourry@gourry.net>
To: Sarthak Sharma <sarthak.sharma@arm.com>
Cc: linux-kselftest@vger.kernel.org, linux-mm@kvack.org,
	 linux-kernel@vger.kernel.org, kernel-team@meta.com,
	akpm@linux-foundation.org,  david@kernel.org, ljs@kernel.org,
	liam@infradead.org, vbabka@kernel.org,  rppt@kernel.org,
	surenb@google.com, mhocko@suse.com, shuah@kernel.org
Subject: Re: [PATCH 3/8] selftests/mm: check MADV_COLD and MADV_PAGEOUT range handling
Date: Fri, 16 Oct 2026 10:07:20 -0400	[thread overview]
Message-ID: <atIvbzEGUcyCqdC7@gourry-fedora-PF4VCD3F> (raw)
In-Reply-To: <atIkDw9r6CVDJaG7@gourry-fedora-PF4VCD3F>

On Fri, Oct 16, 2026 at 09:22:54AM -0400, Gregory Price wrote:
> On Fri, Oct 16, 2026 at 05:47:57PM +0530, Sarthak Sharma wrote:
> > > +/*
> > > + * An unmapped gap in the range makes either hint fail with ENOMEM, after
> > > + * advising the mapped pages on both sides of the gap. Only MADV_PAGEOUT has
> > > + * an effect on those pages that the test can observe.
> > > + */
> > > +TEST_F(madvise, unmapped_gap_fails_after_advising)
> > > +{
> > > +	char *mapping, *control;
> > > +	bool swapped = false;
> > > +	int retry;
> > > +
> > > +	pin_to_current_cpu(_metadata);
> > > +
> > > +	/* Make sure MADV_PAGEOUT can swap at all before relying on it. */
> > > +	control = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
> > > +		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > +	ASSERT_NE(control, MAP_FAILED);
> > > +	control[0] = 1;
> > > +	if (!pageout_until_swapped(_metadata, control, page_size,
> > > +				   self->pagemap_fd))
> > > +		SKIP(return, "MADV_PAGEOUT did not swap the page");
> > 
> > This test is checking for both MADV_COLD and MADV_PAGEOUT. So if we skip
> > here because we couldn't pageout, then we don't test for MADV_COLD
> > behaviour at all. Do we want it this way, or should the tests for both
> > of these be separate?
> > 
> 
> If swap is off, the behavior of COLD and PAGEOUT look pretty much the
> same, and we can't actually test them meaningfully.
> 
> We check that madvise(COLD) returns -1, but that's also largely
> meaningless if this initial pageout_until_swapped() call fails.
> 
> At least that's my thought.  If you think I should split it, I can.
> 

on second look, i will just move the cold checks above the rest, since
they're mostly checking failure paths.  those are mostly unchanging
regardless of swap status.

so basically no need to split, just a mild reorder.

~Gregory


  reply	other threads:[~2026-10-16 14:07 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 17:57 [PATCH 0/8] selftests/mm: add MADV_COLD and MADV_PAGEOUT contract tests Gregory Price (Meta)
2026-10-14 17:57 ` [PATCH 1/8] " Gregory Price (Meta)
2026-10-19  4:59   ` Sarthak Sharma
2026-10-14 17:57 ` [PATCH 2/8] selftests/mm: check that MADV_PAGEOUT skips holes and swapped pages Gregory Price (Meta)
2026-10-19  5:00   ` Sarthak Sharma
2026-10-14 17:57 ` [PATCH 3/8] selftests/mm: check MADV_COLD and MADV_PAGEOUT range handling Gregory Price (Meta)
2026-10-14 21:31   ` [PATCH] fixup! " Gregory Price (Meta)
2026-10-16 12:17   ` [PATCH 3/8] " Sarthak Sharma
2026-10-16 13:22     ` Gregory Price
2026-10-16 14:07       ` Gregory Price [this message]
2026-10-19  4:48         ` Sarthak Sharma
2026-10-14 17:57 ` [PATCH 4/8] selftests/mm: check that MADV_COLD and MADV_PAGEOUT reject hugetlb Gregory Price (Meta)
2026-10-19  5:02   ` Sarthak Sharma
2026-10-14 17:57 ` [PATCH 5/8] selftests/mm: check that MADV_COLD and MADV_PAGEOUT preserve shared memory Gregory Price (Meta)
2026-10-19  5:13   ` Sarthak Sharma
2026-10-14 17:57 ` [PATCH 6/8] selftests/mm: check MADV_COLD and MADV_PAGEOUT on partly advised THPs Gregory Price (Meta)
2026-10-19  5:16   ` Sarthak Sharma
2026-10-14 17:57 ` [PATCH 7/8] selftests/mm: check MADV_PAGEOUT file permissions Gregory Price (Meta)
2026-10-16 12:18   ` Sarthak Sharma
2026-10-16 13:48     ` Gregory Price
2026-10-19  4:51       ` Sarthak Sharma
2026-10-14 17:57 ` [PATCH 8/8] selftests/mm: check that unauthorized MADV_PAGEOUT skips large file folios Gregory Price (Meta)
2026-10-14 21:26   ` Gregory Price
2026-10-19  5:19   ` Sarthak Sharma

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=atIvbzEGUcyCqdC7@gourry-fedora-PF4VCD3F \
    --to=gourry@gourry.net \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=rppt@kernel.org \
    --cc=sarthak.sharma@arm.com \
    --cc=shuah@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.