2019-11-13 04:28:53

by John Hubbard

Subject: [PATCH v4 20/23] mm/gup_benchmark: use proper FOLL_WRITE flags instead of hard-coding "1"

Fix the gup benchmark flags to use the symbolic FOLL_WRITE,
instead of a hard-coded "1" value.

Also, clean up the filtering of gup flags a little, by just doing
it once before issuing any of the get_user_pages*() calls. This
makes it harder to overlook, instead of having little "gup_flags & 1"
phrases in the function calls.

Signed-off-by: John Hubbard <[email protected]>
---
mm/gup_benchmark.c | 9 ++++++---
tools/testing/selftests/vm/gup_benchmark.c | 6 +++++-
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
index 7dd602d7f8db..7fc44d25eca7 100644
--- a/mm/gup_benchmark.c
+++ b/mm/gup_benchmark.c
@@ -48,18 +48,21 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
nr = (next - addr) / PAGE_SIZE;
}

+ /* Filter out most gup flags: only allow a tiny subset here: */
+ gup->flags &= FOLL_WRITE;
+
switch (cmd) {
case GUP_FAST_BENCHMARK:
- nr = get_user_pages_fast(addr, nr, gup->flags & 1,
+ nr = get_user_pages_fast(addr, nr, gup->flags,
pages + i);
break;
case GUP_LONGTERM_BENCHMARK:
nr = get_user_pages(addr, nr,
- (gup->flags & 1) | FOLL_LONGTERM,
+ gup->flags | FOLL_LONGTERM,
pages + i, NULL);
break;
case GUP_BENCHMARK:
- nr = get_user_pages(addr, nr, gup->flags & 1, pages + i,
+ nr = get_user_pages(addr, nr, gup->flags, pages + i,
NULL);
break;
default:
diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_benchmark.c
index 485cf06ef013..389327e9b30a 100644
--- a/tools/testing/selftests/vm/gup_benchmark.c
+++ b/tools/testing/selftests/vm/gup_benchmark.c
@@ -18,6 +18,9 @@
#define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
#define GUP_BENCHMARK _IOWR('g', 3, struct gup_benchmark)

+/* Just the flags we need, copied from mm.h: */
+#define FOLL_WRITE 0x01 /* check pte is writable */
+
struct gup_benchmark {
__u64 get_delta_usec;
__u64 put_delta_usec;
@@ -85,7 +88,8 @@ int main(int argc, char **argv)
}

gup.nr_pages_per_call = nr_pages;
- gup.flags = write;
+ if (write)
+ gup.flags |= FOLL_WRITE;

fd = open("/sys/kernel/debug/gup_benchmark", O_RDWR);
if (fd == -1)
--
2.24.0

2019-11-13 19:29:06

by Ira Weiny

Subject: Re: [PATCH v4 20/23] mm/gup_benchmark: use proper FOLL_WRITE flags instead of hard-coding "1"

On Tue, Nov 12, 2019 at 08:27:07PM -0800, John Hubbard wrote:
> Fix the gup benchmark flags to use the symbolic FOLL_WRITE,
> instead of a hard-coded "1" value.
>
> Also, clean up the filtering of gup flags a little, by just doing
> it once before issuing any of the get_user_pages*() calls. This
> makes it harder to overlook, instead of having little "gup_flags & 1"
> phrases in the function calls.
>
> Signed-off-by: John Hubbard <[email protected]>

Reviewed-by: Ira Weiny <[email protected]>

> ---
> mm/gup_benchmark.c | 9 ++++++---
> tools/testing/selftests/vm/gup_benchmark.c | 6 +++++-
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
> index 7dd602d7f8db..7fc44d25eca7 100644
> --- a/mm/gup_benchmark.c
> +++ b/mm/gup_benchmark.c
> @@ -48,18 +48,21 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
> nr = (next - addr) / PAGE_SIZE;
> }
>
> + /* Filter out most gup flags: only allow a tiny subset here: */
> + gup->flags &= FOLL_WRITE;
> +
> switch (cmd) {
> case GUP_FAST_BENCHMARK:
> - nr = get_user_pages_fast(addr, nr, gup->flags & 1,
> + nr = get_user_pages_fast(addr, nr, gup->flags,
> pages + i);
> break;
> case GUP_LONGTERM_BENCHMARK:
> nr = get_user_pages(addr, nr,
> - (gup->flags & 1) | FOLL_LONGTERM,
> + gup->flags | FOLL_LONGTERM,
> pages + i, NULL);
> break;
> case GUP_BENCHMARK:
> - nr = get_user_pages(addr, nr, gup->flags & 1, pages + i,
> + nr = get_user_pages(addr, nr, gup->flags, pages + i,
> NULL);
> break;
> default:
> diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_benchmark.c
> index 485cf06ef013..389327e9b30a 100644
> --- a/tools/testing/selftests/vm/gup_benchmark.c
> +++ b/tools/testing/selftests/vm/gup_benchmark.c
> @@ -18,6 +18,9 @@
> #define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
> #define GUP_BENCHMARK _IOWR('g', 3, struct gup_benchmark)
>
> +/* Just the flags we need, copied from mm.h: */
> +#define FOLL_WRITE 0x01 /* check pte is writable */
> +
> struct gup_benchmark {
> __u64 get_delta_usec;
> __u64 put_delta_usec;
> @@ -85,7 +88,8 @@ int main(int argc, char **argv)
> }
>
> gup.nr_pages_per_call = nr_pages;
> - gup.flags = write;
> + if (write)
> + gup.flags |= FOLL_WRITE;
>
> fd = open("/sys/kernel/debug/gup_benchmark", O_RDWR);
> if (fd == -1)
> --
> 2.24.0
>