[LTP] [PATCH v1] move_pages04: check for "invalid area", "no page mapped" and "shared zero page mapped"

[Jan Stancek]

On Thu, Sep 26, 2024 at 6:51 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 26.09.24 11:09, Jan Stancek wrote:
> > On Thu, Aug 29, 2024 at 4:10 PM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> While the kernel commit d899844e9c98 ("mm: fix status code which
> >> move_pages() returns for zero page") fixed the return value when the
> >> shared zero page was encountered to match what was state in the man page,
> >> it unfortunately also changed the behavior when no page is mapped yet --
> >> when no page was faulted in/populated on demand.
> >>
> >> Then, this test started failing, and we thought we would be testing for
> >> the "zero page" case, but actually we were testing for the "no page mapped"
> >> case, and didn't realize that the kernel commit had unintended side
> >> effects.
> >>
> >> As we are changing the behavior back to return "-ENOENT" in the kernel
> >> for the "no page mapped" case, while still keeping the "shared zero
> >> page" case to return "-EFAULT" the test starts failing again ...
> >>
> >> The man page clearly spells out that the expectation for the zero page is
> >> "-EFAULT", and that "-EFAULT" can also be returned if "the memory area is
> >> not mapped by the process" -- which means that there is no VMA/mmap()
> >> covering that address.
> >>
> >> The man page also documents that "-ENOENT" is returned when "The page is
> >> not present", which includes "there is nothing mapped".
> >>
> >> So let's fix the test and properly testing for all three separate things:
> >> "invalid area/page", "no page mapped" and "shared zero page mapped">
> >>
> >> Cc: Cyril Hrubis <chrubis@suse.cz>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >>
> >> The kernel change[1] that will revert to the documented behavior -- return
> >> -ENOENT when no page is mapped yet -- is not upstream yet, but the
> >> assumption is that it will go upstream in the next merge window, to land
> >> in v6.12.
> >
> > Thanks for patch, looking at the Linus' tree I think this landed already.
>
> Yes, it's upstream.
>
> >
> >>
> >> Consequently, this test will now fail (as expected) on kernels between
> >> v4.3 and current mainline.
> >>
> >> We seemed to have "hacked" the test to make kernels < 4.3 still pass,
> >> even though they were handling zero pages the wrong way.
> >>
> >> Should we add a similar "hack" for kernels >= 4.3 up to the one where
> >> the kernel behavior will change? (likely v6.12)?
> >
> > I'm leaning towards removing the "< 4.3 hack" (in follow-up patch), because
> > on distros that do backports it's going to be even more confusing.
>
> Makes sense, so we would really test against the documented+expected
> behavior.
>
> I will resend with:
>
> (1) This patch, including the proper patch description
> (2) A patch removing the < 4.3 hack
> (3) A patch to convert this test to the new API
>
>
> Sounds good?

Sounds good to me.