On Sun, Jan 24, 2021 at 6:18 PM John Hubbard <jhubbard@xxxxxxxxxx> 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
> to be passing my set of "run a bunch of different gup_test options" here,
> so feel free to add:
>
> Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>
Thank you!
Pasha
>
> thanks,
> --
> John Hubbard
> NVIDIA
>
> > diff --git a/mm/gup_test.c b/mm/gup_test.c
> > index a6ed1c877679..d974dec19e1c 100644
> > --- a/mm/gup_test.c
> > +++ b/mm/gup_test.c
> > @@ -52,6 +52,12 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
> >
> > dump_page(page, "gup_test failure");
> > break;
> > + } else if (cmd == PIN_LONGTERM_BENCHMARK &&
> > + WARN(!is_pinnable_page(page),
> > + "pages[%lu] is NOT pinnable but pinned\n",
> > + i)) {
> > + dump_page(page, "gup_test failure");
> > + break;
> > }
> > }
> > break;
> > diff --git a/tools/testing/selftests/vm/gup_test.c b/tools/testing/selftests/vm/gup_test.c
> > index 943cc2608dc2..1e662d59c502 100644
> > --- a/tools/testing/selftests/vm/gup_test.c
> > +++ b/tools/testing/selftests/vm/gup_test.c
> > @@ -13,6 +13,7 @@
> >
> > /* Just the flags we need, copied from mm.h: */
> > #define FOLL_WRITE 0x01 /* check pte is writable */
> > +#define FOLL_TOUCH 0x02 /* mark page accessed */
> >
> > static char *cmd_to_str(unsigned long cmd)
> > {
> > @@ -39,11 +40,11 @@ int main(int argc, char **argv)
> > unsigned long size = 128 * MB;
> > int i, fd, filed, opt, nr_pages = 1, thp = -1, repeats = 1, write = 1;
> > unsigned long cmd = GUP_FAST_BENCHMARK;
> > - int flags = MAP_PRIVATE;
> > + int flags = MAP_PRIVATE, touch = 0;
> > char *file = "/dev/zero";
> > char *p;
> >
> > - while ((opt = getopt(argc, argv, "m:r:n:F:f:abctTLUuwWSHp")) != -1) {
> > + while ((opt = getopt(argc, argv, "m:r:n:F:f:abctTLUuwWSHpz")) != -1) {
> > switch (opt) {
> > case 'a':
> > cmd = PIN_FAST_BENCHMARK;
> > @@ -110,6 +111,10 @@ int main(int argc, char **argv)
> > case 'H':
> > flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
> > break;
> > + case 'z':
> > + /* fault pages in gup, do not fault in userland */
> > + touch = 1;
> > + break;
> > default:
> > return -1;
> > }
> > @@ -167,8 +172,18 @@ int main(int argc, char **argv)
> > else if (thp == 0)
> > madvise(p, size, MADV_NOHUGEPAGE);
> >
> > - for (; (unsigned long)p < gup.addr + size; p += PAGE_SIZE)
> > - p[0] = 0;
> > + /*
> > + * FOLL_TOUCH, in gup_test, is used as an either/or case: either
> > + * fault pages in from the kernel via FOLL_TOUCH, or fault them
> > + * in here, from user space. This allows comparison of performance
> > + * between those two cases.
> > + */
> > + if (touch) {
> > + gup.gup_flags |= FOLL_TOUCH;
> > + } else {
> > + for (; (unsigned long)p < gup.addr + size; p += PAGE_SIZE)
> > + p[0] = 0;
> > + }
> >
> > /* Only report timing information on the *_BENCHMARK commands: */
> > if ((cmd == PIN_FAST_BENCHMARK) || (cmd == GUP_FAST_BENCHMARK) ||
> >
>
