Re: [PATCH 7/8] selftests/mm: check MADV_PAGEOUT file permissions

[Gregory Price <gourry@gourry.net>]

On Fri, Oct 16, 2026 at 05:48:11PM +0530, Sarthak Sharma wrote:
> > +/*
> > + * The owner of a file may page out its clean page-cache pages. Run as an
> > + * unprivileged owner, so that capabilities cannot stand in for ownership.
> > + */
> > +TEST_F(madvise, pageout_evicts_owned_file_page)
> > +{
> > +   char *mapping, *control;
> > +
> > +   if (!geteuid() && !drop_privileges())
> > +      SKIP(return, "could not drop privileges");
> > +   pin_to_current_cpu(_metadata);
> 
> Is there a reason why we are pinning to the current CPU here and not in
> the next two tests?

Oversight on my part.  I added it where i got a failure, but didn't
carry it forward to the other tests.

> > +   ASSERT_EQ(mapping[0], 0);
> > +   ASSERT_EQ(mapping[page_size], 0);
> > +   mapping[0] = 1;
> 
> If both pages are on CPU 1's per CPU LRU batch here and then hte task
> migrates to CPU 2 here
> 
> > +
> > +   ASSERT_EQ(madvise(mapping, 2 * page_size, MADV_PAGEOUT), 0);
> 
> Then this can cause lru_add_drain() on CPU 2, and therefore both pages
> do not reach the main LRU list. Thus they won't be reclaimed and the
> test can skip here since the CoW page wasn't swapped.

I think there are other corner cases here as well. So yes i'll set up
thei pinning - probably worth it to just pin the entire suite of tests.

My original set had a bunch of this tested more explicitly, but this is
really implementation details that are not worth the additional test
complexity.  I think we can just rid ourselves of that and just pin
every test and make sure it does what we expect in that case.

> 
> > +   ASSERT_TRUE(pagemap_is_populated(self->pagemap_fd,
> > +                mapping + page_size));
> > +   swapped = pagemap_is_swapped(self->pagemap_fd, mapping);
> > +   ASSERT_EQ(mapping[0], 1);
> > +   ASSERT_EQ(mapping[page_size], 0);
> > +   if (!swapped)
> > +      SKIP(return, "MADV_PAGEOUT did not swap the COW page");
> 
> Also, here we skip if we couldn't swap out the CoW page. Won't it make
> more sense to check with pageout_until_swapped() first if pages can be
> skipped, and then fail here if the CoW page wasn't skipped after all the
> retries, similar to how some other tests implement it?
> 

Ah yeah good catch.  I'll adjust like this:

   /* Ensure pageout-swap actually works first with a control page */
   control[0] = 1;
   if (!pageout_until_swapped(_metadata, control, page_size,
                              self->pagemap_fd))
      SKIP(return, "MADV_PAGEOUT did not swap the control page");

   /* Run the actual test */
   ASSERT_EQ(mapping[0], 0);  
   ASSERT_EQ(mapping[page_size], 0);
   mapping[0] = 1;

   ASSERT_TRUE(pageout_until_swapped(_metadata, mapping, 2 * page_size,
                                     self->pagemap_fd));
   ASSERT_TRUE(pagemap_is_populated(self->pagemap_fd,
                                    mapping + page_size));
   ASSERT_EQ(mapping[0], 1);         
   ASSERT_EQ(mapping[page_size], 0);

~Gregory