On Thu, May 15, 2025 at 02:46:41PM -0400, Zi Yan wrote:
> On 15 May 2025, at 14:41, Lorenzo Stoakes wrote:
>
> > Ah you got to this first :) thanks!
> >
> > Could you do this with a cover letter though? It's really weird to have 2/2
> > reply to 1/2, I know sometimes people do that, but it's just odd, and it'd 
> > be
> > good to have an overview, thanks!
> >
> > On Thu, May 15, 2025 at 02:23:32PM -0400, Zi Yan wrote:
> >> When userfaultfd is not compiled into kernel, userfaultfd() returns -1,
> >> causing uffd tests in madv_guard fail. Skip the tests instead.
> >
> > 'madv_guard'? I'd just say the guard_regions.uffd test to fail.
>
> Sure. Will change it.
> >
> >>
> >> Signed-off-by: Zi Yan <z...@nvidia.com>

Given I was being an idiot below, now the patch is fine as-is, just resend
with the nitty commit message change and cover letter as a v2 and we should
be good :)

Reviewed-by: Lorenzo Stoakes <lorenzo.stoa...@oracle.com>

> >> ---
> >>  tools/testing/selftests/mm/guard-regions.c | 17 +++++++++++++++--
> >>  1 file changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/mm/guard-regions.c 
> >> b/tools/testing/selftests/mm/guard-regions.c
> >> index 0cd9d236649d..93af3d3760f9 100644
> >> --- a/tools/testing/selftests/mm/guard-regions.c
> >> +++ b/tools/testing/selftests/mm/guard-regions.c
> >> @@ -1453,8 +1453,21 @@ TEST_F(guard_regions, uffd)
> >>
> >>    /* Set up uffd. */
> >>    uffd = userfaultfd(0);
> >> -  if (uffd == -1 && errno == EPERM)
> >> -          ksft_exit_skip("No userfaultfd permissions, try running as 
> >> root.\n");
> >
> > Let's just make this all part of the same switch please!
>
> What do you mean? EPERM is handled in the switch-case below.

Oh man, I'm the biggest idiot on Earth haha!

For some reason I read these '-'s as '+'s :))))

Yes please ignore the above, I therefore - like your approach - and am good
with it.

>
> >
> > And while I originally used ksft_exit_skip(), I think we can just use the
> > SKIP(return, ...) form here just fine to keep it consistent.
>
> Right. I am using SKIP below, since when I ran it, ksft_exit_skip()
> makes the whole test message inconsistent.

Yes, your confusion is warranted, I apparently can't read... :>) and
indeed, agreed.

Thanks for doing this!

>
> >
> >> +  if (uffd == -1) {
> >> +          switch (errno) {
> >> +          case EPERM:
> >> +                  SKIP(return, "No userfaultfd permissions, try running 
> >> as root.");
> >> +                  break;
> >> +          case ENOSYS:
> >> +                  SKIP(return, "userfaultfd is not supported/not 
> >> enabled.");
> >> +                  break;
> >> +          default:
> >> +                  ksft_exit_fail_msg("userfaultfd failed with %s\n",
> >> +                                     strerror(errno));
> >> +                  break;
> >> +          }
> >> +  }
> >> +
> >>    ASSERT_NE(uffd, -1);
> >>
> >>    ASSERT_EQ(ioctl(uffd, UFFDIO_API, &api), 0);
> >> --
> >> 2.47.2
> >>
> >
> > Thanks!
>
>
> --
> Best Regards,
> Yan, Zi

Reply via email to