* [PATCH 2/6] mm/gup_benchmark: Add additional pinning methods
2018-10-10 19:56 [PATCH 1/6] mm/gup_benchmark: Time put_page Keith Busch
@ 2018-10-10 19:56 ` Keith Busch
2018-10-10 19:56 ` [PATCH 3/6] tools/gup_benchmark: Fix 'write' flag usage Keith Busch
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2018-10-10 19:56 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Kirill Shutemov, Dave Hansen, Dan Williams, Andrew Morton, Keith Busch
This patch provides new gup benchmark ioctl commands to run different
user page pinning methods, get_user_pages_longterm and get_user_pages,
in addition to the existing get_user_pages_fast.
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
mm/gup_benchmark.c | 28 ++++++++++++++++++++++++++--
tools/testing/selftests/vm/gup_benchmark.c | 13 +++++++++++--
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
index b344abd6e8e4..ab103a018627 100644
--- a/mm/gup_benchmark.c
+++ b/mm/gup_benchmark.c
@@ -6,6 +6,8 @@
#include <linux/debugfs.h>
#define GUP_FAST_BENCHMARK _IOWR('g', 1, struct gup_benchmark)
+#define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
+#define GUP_BENCHMARK _IOWR('g', 3, struct gup_benchmark)
struct gup_benchmark {
__u64 get_delta_usec;
@@ -42,7 +44,23 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
nr = (next - addr) / PAGE_SIZE;
}
- nr = get_user_pages_fast(addr, nr, gup->flags & 1, pages + i);
+ switch (cmd) {
+ case GUP_FAST_BENCHMARK:
+ nr = get_user_pages_fast(addr, nr, gup->flags & 1,
+ pages + i);
+ break;
+ case GUP_LONGTERM_BENCHMARK:
+ nr = get_user_pages_longterm(addr, nr, gup->flags & 1,
+ pages + i, NULL);
+ break;
+ case GUP_BENCHMARK:
+ nr = get_user_pages(addr, nr, gup->flags & 1, pages + i,
+ NULL);
+ break;
+ default:
+ return -1;
+ }
+
if (nr <= 0)
break;
i += nr;
@@ -71,8 +89,14 @@ static long gup_benchmark_ioctl(struct file *filep, unsigned int cmd,
struct gup_benchmark gup;
int ret;
- if (cmd != GUP_FAST_BENCHMARK)
+ switch (cmd) {
+ case GUP_FAST_BENCHMARK:
+ case GUP_LONGTERM_BENCHMARK:
+ case GUP_BENCHMARK:
+ break;
+ default:
return -EINVAL;
+ }
if (copy_from_user(&gup, (void __user *)arg, sizeof(gup)))
return -EFAULT;
diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_benchmark.c
index bdcb97acd0ac..c2f785ded9b9 100644
--- a/tools/testing/selftests/vm/gup_benchmark.c
+++ b/tools/testing/selftests/vm/gup_benchmark.c
@@ -15,6 +15,8 @@
#define PAGE_SIZE sysconf(_SC_PAGESIZE)
#define GUP_FAST_BENCHMARK _IOWR('g', 1, struct gup_benchmark)
+#define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
+#define GUP_BENCHMARK _IOWR('g', 3, struct gup_benchmark)
struct gup_benchmark {
__u64 get_delta_usec;
@@ -30,9 +32,10 @@ int main(int argc, char **argv)
struct gup_benchmark gup;
unsigned long size = 128 * MB;
int i, fd, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
+ int cmd = GUP_FAST_BENCHMARK;
char *p;
- while ((opt = getopt(argc, argv, "m:r:n:tT")) != -1) {
+ while ((opt = getopt(argc, argv, "m:r:n:tTLU")) != -1) {
switch (opt) {
case 'm':
size = atoi(optarg) * MB;
@@ -49,6 +52,12 @@ int main(int argc, char **argv)
case 'T':
thp = 0;
break;
+ case 'L':
+ cmd = GUP_LONGTERM_BENCHMARK;
+ break;
+ case 'U':
+ cmd = GUP_BENCHMARK;
+ break;
case 'w':
write = 1;
default:
@@ -79,7 +88,7 @@ int main(int argc, char **argv)
for (i = 0; i < repeats; i++) {
gup.size = size;
- if (ioctl(fd, GUP_FAST_BENCHMARK, &gup))
+ if (ioctl(fd, cmd, &gup))
perror("ioctl"), exit(1);
printf("Time: get:%lld put:%lld us", gup.get_delta_usec,
--
2.14.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/6] tools/gup_benchmark: Fix 'write' flag usage
2018-10-10 19:56 [PATCH 1/6] mm/gup_benchmark: Time put_page Keith Busch
2018-10-10 19:56 ` [PATCH 2/6] mm/gup_benchmark: Add additional pinning methods Keith Busch
@ 2018-10-10 19:56 ` Keith Busch
2018-10-10 19:56 ` [PATCH 4/6] tools/gup_benchmark: Allow user specified file Keith Busch
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2018-10-10 19:56 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Kirill Shutemov, Dave Hansen, Dan Williams, Andrew Morton, Keith Busch
If the '-w' parameter was provided, the benchmark would exit due to a
mssing 'break'.
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
tools/testing/selftests/vm/gup_benchmark.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_benchmark.c
index c2f785ded9b9..b2082df8beb4 100644
--- a/tools/testing/selftests/vm/gup_benchmark.c
+++ b/tools/testing/selftests/vm/gup_benchmark.c
@@ -60,6 +60,7 @@ int main(int argc, char **argv)
break;
case 'w':
write = 1;
+ break;
default:
return -1;
}
--
2.14.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/6] tools/gup_benchmark: Allow user specified file
2018-10-10 19:56 [PATCH 1/6] mm/gup_benchmark: Time put_page Keith Busch
2018-10-10 19:56 ` [PATCH 2/6] mm/gup_benchmark: Add additional pinning methods Keith Busch
2018-10-10 19:56 ` [PATCH 3/6] tools/gup_benchmark: Fix 'write' flag usage Keith Busch
@ 2018-10-10 19:56 ` Keith Busch
2018-10-10 22:31 ` Andrew Morton
2018-10-10 19:56 ` [PATCH 5/6] tools/gup_benchmark: Add MAP_SHARED option Keith Busch
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Keith Busch @ 2018-10-10 19:56 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Kirill Shutemov, Dave Hansen, Dan Williams, Andrew Morton, Keith Busch
This patch allows a user to specify a file to map by adding a new option,
'-f', providing a means to test various file backings.
If not specified, the benchmark will use a private mapping of /dev/zero,
which produces an anonymous mapping as before.
Cc: Kirill Shutemov <kirill.shutemov@linux.intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
tools/testing/selftests/vm/gup_benchmark.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_benchmark.c
index b2082df8beb4..b675a3d60975 100644
--- a/tools/testing/selftests/vm/gup_benchmark.c
+++ b/tools/testing/selftests/vm/gup_benchmark.c
@@ -31,11 +31,12 @@ int main(int argc, char **argv)
{
struct gup_benchmark gup;
unsigned long size = 128 * MB;
- int i, fd, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
+ int i, fd, filed, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
int cmd = GUP_FAST_BENCHMARK;
+ char *file = "/dev/zero";
char *p;
- while ((opt = getopt(argc, argv, "m:r:n:tTLU")) != -1) {
+ while ((opt = getopt(argc, argv, "m:r:n:f:tTLU")) != -1) {
switch (opt) {
case 'm':
size = atoi(optarg) * MB;
@@ -61,11 +62,18 @@ int main(int argc, char **argv)
case 'w':
write = 1;
break;
+ case 'f':
+ file = optarg;
+ break;
default:
return -1;
}
}
+ filed = open(file, O_RDWR|O_CREAT);
+ if (filed < 0)
+ perror("open"), exit(filed);
+
gup.nr_pages_per_call = nr_pages;
gup.flags = write;
@@ -73,8 +81,7 @@ int main(int argc, char **argv)
if (fd == -1)
perror("open"), exit(1);
- p = mmap(NULL, size, PROT_READ | PROT_WRITE,
- MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ p = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, filed, 0);
if (p == MAP_FAILED)
perror("mmap"), exit(1);
gup.addr = (unsigned long)p;
--
2.14.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/6] tools/gup_benchmark: Allow user specified file
2018-10-10 19:56 ` [PATCH 4/6] tools/gup_benchmark: Allow user specified file Keith Busch
@ 2018-10-10 22:31 ` Andrew Morton
2018-10-10 22:42 ` Keith Busch
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2018-10-10 22:31 UTC (permalink / raw)
To: Keith Busch
Cc: linux-mm, linux-kernel, Kirill Shutemov, Dave Hansen, Dan Williams
On Wed, 10 Oct 2018 13:56:03 -0600 Keith Busch <keith.busch@intel.com> wrote:
> This patch allows a user to specify a file to map by adding a new option,
> '-f', providing a means to test various file backings.
>
> If not specified, the benchmark will use a private mapping of /dev/zero,
> which produces an anonymous mapping as before.
>
> ...
>
> --- a/tools/testing/selftests/vm/gup_benchmark.c
> +++ b/tools/testing/selftests/vm/gup_benchmark.c
>
> ...
>
> @@ -61,11 +62,18 @@ int main(int argc, char **argv)
> case 'w':
> write = 1;
> break;
> + case 'f':
> + file = optarg;
> + break;
> default:
> return -1;
> }
> }
>
> + filed = open(file, O_RDWR|O_CREAT);
> + if (filed < 0)
> + perror("open"), exit(filed);
Ick. Like this, please:
--- a/tools/testing/selftests/vm/gup_benchmark.c~tools-gup_benchmark-allow-user-specified-file-fix
+++ a/tools/testing/selftests/vm/gup_benchmark.c
@@ -71,8 +71,10 @@ int main(int argc, char **argv)
}
filed = open(file, O_RDWR|O_CREAT);
- if (filed < 0)
- perror("open"), exit(filed);
+ if (filed < 0) {
+ perror("open");
+ exit(filed);
+ }
gup.nr_pages_per_call = nr_pages;
gup.flags = write;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/6] tools/gup_benchmark: Allow user specified file
2018-10-10 22:31 ` Andrew Morton
@ 2018-10-10 22:42 ` Keith Busch
0 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2018-10-10 22:42 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, Kirill Shutemov, Dave Hansen, Dan Williams
On Wed, Oct 10, 2018 at 03:31:01PM -0700, Andrew Morton wrote:
> On Wed, 10 Oct 2018 13:56:03 -0600 Keith Busch <keith.busch@intel.com> wrote:
> > + filed = open(file, O_RDWR|O_CREAT);
> > + if (filed < 0)
> > + perror("open"), exit(filed);
>
> Ick. Like this, please:
Yeah, I agree. I just copied the style this file had been using in other
error cases, but I still find it less readable than your recommendation.
> --- a/tools/testing/selftests/vm/gup_benchmark.c~tools-gup_benchmark-allow-user-specified-file-fix
> +++ a/tools/testing/selftests/vm/gup_benchmark.c
> @@ -71,8 +71,10 @@ int main(int argc, char **argv)
> }
>
> filed = open(file, O_RDWR|O_CREAT);
> - if (filed < 0)
> - perror("open"), exit(filed);
> + if (filed < 0) {
> + perror("open");
> + exit(filed);
> + }
>
> gup.nr_pages_per_call = nr_pages;
> gup.flags = write;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/6] tools/gup_benchmark: Add MAP_SHARED option
2018-10-10 19:56 [PATCH 1/6] mm/gup_benchmark: Time put_page Keith Busch
` (2 preceding siblings ...)
2018-10-10 19:56 ` [PATCH 4/6] tools/gup_benchmark: Allow user specified file Keith Busch
@ 2018-10-10 19:56 ` Keith Busch
2018-10-10 19:56 ` [PATCH 6/6] tools/gup_benchmark: Add MAP_HUGETLB option Keith Busch
2018-10-10 22:26 ` [PATCH 1/6] mm/gup_benchmark: Time put_page Andrew Morton
5 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2018-10-10 19:56 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Kirill Shutemov, Dave Hansen, Dan Williams, Andrew Morton, Keith Busch
This patch adds a new benchmark option, -S, to request MAP_SHARED. This
can be used to compare with MAP_PRIVATE, or for files that require this
option, like dax.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
tools/testing/selftests/vm/gup_benchmark.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_benchmark.c
index b675a3d60975..24528b54549d 100644
--- a/tools/testing/selftests/vm/gup_benchmark.c
+++ b/tools/testing/selftests/vm/gup_benchmark.c
@@ -32,11 +32,11 @@ int main(int argc, char **argv)
struct gup_benchmark gup;
unsigned long size = 128 * MB;
int i, fd, filed, opt, nr_pages = 1, thp = -1, repeats = 1, write = 0;
- int cmd = GUP_FAST_BENCHMARK;
+ int cmd = GUP_FAST_BENCHMARK, flags = MAP_PRIVATE;
char *file = "/dev/zero";
char *p;
- while ((opt = getopt(argc, argv, "m:r:n:f:tTLU")) != -1) {
+ while ((opt = getopt(argc, argv, "m:r:n:f:tTLUS")) != -1) {
switch (opt) {
case 'm':
size = atoi(optarg) * MB;
@@ -65,6 +65,10 @@ int main(int argc, char **argv)
case 'f':
file = optarg;
break;
+ case 'S':
+ flags &= ~MAP_PRIVATE;
+ flags |= MAP_SHARED;
+ break;
default:
return -1;
}
@@ -81,7 +85,7 @@ int main(int argc, char **argv)
if (fd == -1)
perror("open"), exit(1);
- p = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, filed, 0);
+ p = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, filed, 0);
if (p == MAP_FAILED)
perror("mmap"), exit(1);
gup.addr = (unsigned long)p;
--
2.14.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/6] tools/gup_benchmark: Add MAP_HUGETLB option
2018-10-10 19:56 [PATCH 1/6] mm/gup_benchmark: Time put_page Keith Busch
` (3 preceding siblings ...)
2018-10-10 19:56 ` [PATCH 5/6] tools/gup_benchmark: Add MAP_SHARED option Keith Busch
@ 2018-10-10 19:56 ` Keith Busch
2018-10-10 22:26 ` [PATCH 1/6] mm/gup_benchmark: Time put_page Andrew Morton
5 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2018-10-10 19:56 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Kirill Shutemov, Dave Hansen, Dan Williams, Andrew Morton, Keith Busch
This patch adds a new option, '-H', to the gup benchmark to help compare how
hugetlb mapping pages compare with the default.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
tools/testing/selftests/vm/gup_benchmark.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vm/gup_benchmark.c b/tools/testing/selftests/vm/gup_benchmark.c
index 24528b54549d..c7b5934c6d7f 100644
--- a/tools/testing/selftests/vm/gup_benchmark.c
+++ b/tools/testing/selftests/vm/gup_benchmark.c
@@ -36,7 +36,7 @@ int main(int argc, char **argv)
char *file = "/dev/zero";
char *p;
- while ((opt = getopt(argc, argv, "m:r:n:f:tTLUS")) != -1) {
+ while ((opt = getopt(argc, argv, "m:r:n:f:tTLUSH")) != -1) {
switch (opt) {
case 'm':
size = atoi(optarg) * MB;
@@ -69,6 +69,9 @@ int main(int argc, char **argv)
flags &= ~MAP_PRIVATE;
flags |= MAP_SHARED;
break;
+ case 'H':
+ flags |= MAP_HUGETLB;
+ break;
default:
return -1;
}
--
2.14.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] mm/gup_benchmark: Time put_page
2018-10-10 19:56 [PATCH 1/6] mm/gup_benchmark: Time put_page Keith Busch
` (4 preceding siblings ...)
2018-10-10 19:56 ` [PATCH 6/6] tools/gup_benchmark: Add MAP_HUGETLB option Keith Busch
@ 2018-10-10 22:26 ` Andrew Morton
2018-10-10 22:28 ` Keith Busch
5 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2018-10-10 22:26 UTC (permalink / raw)
To: Keith Busch
Cc: linux-mm, linux-kernel, Kirill Shutemov, Dave Hansen, Dan Williams
On Wed, 10 Oct 2018 13:56:00 -0600 Keith Busch <keith.busch@intel.com> wrote:
> We'd like to measure time to unpin user pages, so this adds a second
> benchmark timer on put_page, separate from get_page.
>
> Adding the field will breaks this ioctl ABI, but should be okay since
> this an in-tree kernel selftest.
>
> ...
>
> --- a/mm/gup_benchmark.c
> +++ b/mm/gup_benchmark.c
> @@ -8,7 +8,8 @@
> #define GUP_FAST_BENCHMARK _IOWR('g', 1, struct gup_benchmark)
>
> struct gup_benchmark {
> - __u64 delta_usec;
> + __u64 get_delta_usec;
> + __u64 put_delta_usec;
> __u64 addr;
> __u64 size;
> __u32 nr_pages_per_call;
If we move put_delta_usec to the end of this struct, the ABI remains
back-compatible?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] mm/gup_benchmark: Time put_page
2018-10-10 22:26 ` [PATCH 1/6] mm/gup_benchmark: Time put_page Andrew Morton
@ 2018-10-10 22:28 ` Keith Busch
2018-10-10 22:41 ` Andrew Morton
0 siblings, 1 reply; 12+ messages in thread
From: Keith Busch @ 2018-10-10 22:28 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, Kirill Shutemov, Dave Hansen, Dan Williams
On Wed, Oct 10, 2018 at 03:26:55PM -0700, Andrew Morton wrote:
> On Wed, 10 Oct 2018 13:56:00 -0600 Keith Busch <keith.busch@intel.com> wrote:
>
> > We'd like to measure time to unpin user pages, so this adds a second
> > benchmark timer on put_page, separate from get_page.
> >
> > Adding the field will breaks this ioctl ABI, but should be okay since
> > this an in-tree kernel selftest.
> >
> > ...
> >
> > --- a/mm/gup_benchmark.c
> > +++ b/mm/gup_benchmark.c
> > @@ -8,7 +8,8 @@
> > #define GUP_FAST_BENCHMARK _IOWR('g', 1, struct gup_benchmark)
> >
> > struct gup_benchmark {
> > - __u64 delta_usec;
> > + __u64 get_delta_usec;
> > + __u64 put_delta_usec;
> > __u64 addr;
> > __u64 size;
> > __u32 nr_pages_per_call;
>
> If we move put_delta_usec to the end of this struct, the ABI remains
> back-compatible?
If the kernel writes to a new value appended to the end of the struct,
and the application allocated the older sized struct, wouldn't that
corrupt the user memory?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] mm/gup_benchmark: Time put_page
2018-10-10 22:28 ` Keith Busch
@ 2018-10-10 22:41 ` Andrew Morton
2018-10-10 22:40 ` Keith Busch
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2018-10-10 22:41 UTC (permalink / raw)
To: Keith Busch
Cc: linux-mm, linux-kernel, Kirill Shutemov, Dave Hansen, Dan Williams
On Wed, 10 Oct 2018 16:28:43 -0600 Keith Busch <keith.busch@intel.com> wrote:
> > > struct gup_benchmark {
> > > - __u64 delta_usec;
> > > + __u64 get_delta_usec;
> > > + __u64 put_delta_usec;
> > > __u64 addr;
> > > __u64 size;
> > > __u32 nr_pages_per_call;
> >
> > If we move put_delta_usec to the end of this struct, the ABI remains
> > back-compatible?
>
> If the kernel writes to a new value appended to the end of the struct,
> and the application allocated the older sized struct, wouldn't that
> corrupt the user memory?
Looks like it. How about we do this while we're breaking it?
--- a/mm/gup_benchmark.c~mm-gup_benchmark-time-put_page-fix
+++ a/mm/gup_benchmark.c
@@ -14,6 +14,7 @@ struct gup_benchmark {
__u64 size;
__u32 nr_pages_per_call;
__u32 flags;
+ __u64 expansion[10]; /* For future use */
};
static int __gup_benchmark_ioctl(unsigned int cmd,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] mm/gup_benchmark: Time put_page
2018-10-10 22:41 ` Andrew Morton
@ 2018-10-10 22:40 ` Keith Busch
0 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2018-10-10 22:40 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, Kirill Shutemov, Dave Hansen, Dan Williams
On Wed, Oct 10, 2018 at 03:41:11PM -0700, Andrew Morton wrote:
> On Wed, 10 Oct 2018 16:28:43 -0600 Keith Busch <keith.busch@intel.com> wrote:
>
> > > > struct gup_benchmark {
> > > > - __u64 delta_usec;
> > > > + __u64 get_delta_usec;
> > > > + __u64 put_delta_usec;
> > > > __u64 addr;
> > > > __u64 size;
> > > > __u32 nr_pages_per_call;
> > >
> > > If we move put_delta_usec to the end of this struct, the ABI remains
> > > back-compatible?
> >
> > If the kernel writes to a new value appended to the end of the struct,
> > and the application allocated the older sized struct, wouldn't that
> > corrupt the user memory?
>
> Looks like it. How about we do this while we're breaking it?
Yep, that sounds good to me!
> --- a/mm/gup_benchmark.c~mm-gup_benchmark-time-put_page-fix
> +++ a/mm/gup_benchmark.c
> @@ -14,6 +14,7 @@ struct gup_benchmark {
> __u64 size;
> __u32 nr_pages_per_call;
> __u32 flags;
> + __u64 expansion[10]; /* For future use */
> };
>
> static int __gup_benchmark_ioctl(unsigned int cmd,
>
^ permalink raw reply [flat|nested] 12+ messages in thread