Re: [PATCH v7 14/14] selftests/vm: test faulting in kernel, and verify pinnable pages

[John Hubbard]

On 1/24/21 3:18 PM, John Hubbard wrote:
> On 1/21/21 7:37 PM, Pavel Tatashin wrote:
> > When pages are pinned they can be faulted in userland and migrated, and
> > they can be faulted right in kernel without migration.
> >
> > In either case, the pinned pages must end-up being pinnable (not movable).
> >
> > Add a new test to gup_test, to help verify that the gup/pup
> > (get_user_pages() / pin_user_pages()) behavior with respect to pinnable
> > and movable pages is reasonable and correct. Specifically, provide a
> > way to:
> >
> > 1) Verify that only "pinnable" pages are pinned. This is checked
> > automatically for you.
> >
> > 2) Verify that gup/pup performance is reasonable. This requires
> > comparing benchmarks between doing gup/pup on pages that have been
> > pre-faulted in from user space, vs. doing gup/pup on pages that are not
> > faulted in until gup/pup time (via FOLL_TOUCH). This decision is
> > controlled with the new -z command line option.
> >
> > Signed-off-by: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
> > ---
> >   mm/gup_test.c                         |  6 ++++++
> >   tools/testing/selftests/vm/gup_test.c | 23 +++++++++++++++++++----
> >   2 files changed, 25 insertions(+), 4 deletions(-)
>
> This also looks good. I do see the WARN_ON_ONCE firing in
> internal_get_user_pages_fast(), when running with *only* the new -z
> option.
>
> I'll poke around the rest of the patchset and see if that is expected
> and normal, but either way the test code itself looks correct and seems

The warning that is firing in internal_get_user_pages_fast() is:

	if (WARN_ON_ONCE(gup_flags & ~(FOLL_WRITE | FOLL_LONGTERM |
				       FOLL_FORCE | FOLL_PIN | FOLL_GET |
				       FOLL_FAST_ONLY)))
		return -EINVAL;

...OK, so this is because "./gup_test -z" invokes get_user_pages_fast(),
which so far does not allow passing in FOLL_TOUCH. Probably because there
is nothing "fast" about touching and possibly faulting in pages. :)

So, again, the test code still looks correct, even though it's possible
to pass in options that run into things that are rejected by gup.c



thanks,
--
John Hubbard
NVIDIA