Message ID | 20170412094112.13181-1-khandual@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
On 04/12/2017 03:11 PM, Anshuman Khandual wrote: > This verifies virtual address mapping below and above the > 128TB range and makes sure that address returned are within > the expected range depending upon the hint passed from the > user space. > > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com> > --- > Tested this on latest ppc-next with Aneesh's yesterday's patch > which can be found at https://patchwork.ozlabs.org/patch/749510/ Hello Aneesh/Michael, Does this look okay as a small singular test for below and above the 128TB range allocations ? Or do you think some thing else needs to be added here as well. Regards Anshuman
Hello, On Wed, 12 Apr 2017 15:11:12 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote: > This verifies virtual address mapping below and above the > 128TB range and makes sure that address returned are within > the expected range depending upon the hint passed from the > user space. This description does not mention anything PPC specific. Can this be generalized to work with Intel L5 enabled kernel as well? Thanks Michal > > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com> > --- > Tested this on latest ppc-next with Aneesh's yesterday's patch > which can be found at https://patchwork.ozlabs.org/patch/749510/ > > tools/testing/selftests/powerpc/mm/.gitignore | 3 +- > tools/testing/selftests/powerpc/mm/Makefile | 2 +- > tools/testing/selftests/powerpc/mm/vaddr.c | 89 > +++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 2 > deletions(-) create mode 100644 > tools/testing/selftests/powerpc/mm/vaddr.c > > diff --git a/tools/testing/selftests/powerpc/mm/.gitignore > b/tools/testing/selftests/powerpc/mm/.gitignore index > e715a3f..af1f31f 100644 --- > a/tools/testing/selftests/powerpc/mm/.gitignore +++ > b/tools/testing/selftests/powerpc/mm/.gitignore @@ -1,4 +1,5 @@ > hugetlb_vs_thp_test > subpage_prot > tempfile > -prot_sao > \ No newline at end of file > +prot_sao > +vaddr > diff --git a/tools/testing/selftests/powerpc/mm/Makefile > b/tools/testing/selftests/powerpc/mm/Makefile index 1cffe54..7345db2 > 100644 --- a/tools/testing/selftests/powerpc/mm/Makefile > +++ b/tools/testing/selftests/powerpc/mm/Makefile > @@ -1,7 +1,7 @@ > noarg: > $(MAKE) -C ../ > > -TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao > +TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao vaddr > TEST_GEN_FILES := tempfile > > include ../../lib.mk > diff --git a/tools/testing/selftests/powerpc/mm/vaddr.c > b/tools/testing/selftests/powerpc/mm/vaddr.c new file mode 100644 > index 0000000..d6485e9 > --- /dev/null > +++ b/tools/testing/selftests/powerpc/mm/vaddr.c > @@ -0,0 +1,89 @@ > +/* > + * Copyright 2017, Anshuman Khandual, IBM Corp. > + * Licensed under GPLv2. > + */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#include <errno.h> > +#include <numaif.h> > +#include <sys/mman.h> > +#include <sys/time.h> > +#include "utils.h" > + > +#define PAGE_SIZE 65536UL /* 64KB */ > +#define MAP_SIZE_16GB 262144UL /* 16GB */ > +#define NR_SLICES_128TB 8192UL /* 128TB */ > +#define NR_SLICES_384TB 24576UL /* 384TB */ > +#define ADDR_MARK_128TB 0x800000000000UL /* Beyond 128TB */ > + > +static char *hind_addr(void) > +{ > + int bits = 48 + rand() % 15; > + > + return (char *) (1UL << bits); > +} > + > +static int validate_addr(char *ptr, int high_addr) > +{ > + unsigned long addr = (unsigned long) ptr; > + > + if (high_addr) { > + if (addr < ADDR_MARK_128TB) { > + printf("Bad address %lx\n", addr); > + return 1; > + } > + return 0; > + } > + > + if (addr > ADDR_MARK_128TB) { > + printf("Bad address %lx\n", addr); > + return 1; > + } > + return 0; > +} > + > +int vaddr(void) > +{ > + char *ptr[NR_SLICES_128TB]; > + char *hptr[NR_SLICES_384TB]; > + char *hint; > + int i; > + > + for (i = 0; i < NR_SLICES_128TB; i++) { > + ptr[i] = mmap(NULL, MAP_SIZE_16GB, PROT_READ | > PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS, > -1, 0); + > + if (ptr[i] == MAP_FAILED) > + break; > + > + if (validate_addr(ptr[i], 0)) > + return 1; > + } > + > + for (i = 0; i < NR_SLICES_384TB; i++) { > + hint = hind_addr(); > + hptr[i] = mmap(hint, MAP_SIZE_16GB, PROT_READ | > PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS, > -1, 0); + > + if (hptr[i] == MAP_FAILED) > + break; > + > + if (validate_addr(hptr[i], 1)) > + return 1; > + } > + > + for (i = 0; i < NR_SLICES_128TB; i++) > + munmap(ptr[i], MAP_SIZE_16GB); > + > + for (i = 0; i < NR_SLICES_384TB; i++) > + munmap(hptr[i], MAP_SIZE_16GB); > + return 0; > +} > + > +int main(int argc, char *argv[]) > +{ > + return test_harness(vaddr, "vaddr-range"); > +}
On 04/15/2017 01:48 AM, Michal Suchanek wrote: > Hello, > > On Wed, 12 Apr 2017 15:11:12 +0530 > Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote: > >> This verifies virtual address mapping below and above the >> 128TB range and makes sure that address returned are within >> the expected range depending upon the hint passed from the >> user space. > > This description does not mention anything PPC specific. Can this be > generalized to work with Intel L5 enabled kernel as well? Hmm, I am not completely aware about the proposed mmap interface for 128TB and beyond till 512TB by Kirill on x86. If both powerpc and x86 will have the same semantics, then this test is generic enough to cover both the architectures and will move it into selftests/vm/ instead of current selftests/powerpc/mm.
Anshuman Khandual <khandual@linux.vnet.ibm.com> writes: > This verifies virtual address mapping below and above the > 128TB range and makes sure that address returned are within > the expected range depending upon the hint passed from the > user space. > > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com> > --- > Tested this on latest ppc-next with Aneesh's yesterday's patch > which can be found at https://patchwork.ozlabs.org/patch/749510/ > > tools/testing/selftests/powerpc/mm/.gitignore | 3 +- > tools/testing/selftests/powerpc/mm/Makefile | 2 +- > tools/testing/selftests/powerpc/mm/vaddr.c | 89 +++++++++++++++++++++++++++ > 3 files changed, 92 insertions(+), 2 deletions(-) > create mode 100644 tools/testing/selftests/powerpc/mm/vaddr.c > > diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore > index e715a3f..af1f31f 100644 > --- a/tools/testing/selftests/powerpc/mm/.gitignore > +++ b/tools/testing/selftests/powerpc/mm/.gitignore > @@ -1,4 +1,5 @@ > hugetlb_vs_thp_test > subpage_prot > tempfile > -prot_sao > \ No newline at end of file > +prot_sao > +vaddr > diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile > index 1cffe54..7345db2 100644 > --- a/tools/testing/selftests/powerpc/mm/Makefile > +++ b/tools/testing/selftests/powerpc/mm/Makefile > @@ -1,7 +1,7 @@ > noarg: > $(MAKE) -C ../ > > -TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao > +TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao vaddr > TEST_GEN_FILES := tempfile > > include ../../lib.mk > diff --git a/tools/testing/selftests/powerpc/mm/vaddr.c b/tools/testing/selftests/powerpc/mm/vaddr.c > new file mode 100644 > index 0000000..d6485e9 > --- /dev/null > +++ b/tools/testing/selftests/powerpc/mm/vaddr.c > @@ -0,0 +1,89 @@ > +/* > + * Copyright 2017, Anshuman Khandual, IBM Corp. > + * Licensed under GPLv2. > + */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#include <errno.h> > +#include <numaif.h> > +#include <sys/mman.h> > +#include <sys/time.h> > +#include "utils.h" > + > +#define PAGE_SIZE 65536UL /* 64KB */ > +#define MAP_SIZE_16GB 262144UL /* 16GB */ can we make it MMAP_CHUNK_SIZE ? > +#define NR_SLICES_128TB 8192UL /* 128TB */ NR_CHUNK_128TB ? > +#define NR_SLICES_384TB 24576UL /* 384TB */ Can you add more comments around that. Those numbers/names have special meaning on powerpc. I guess you are not doing anything related that. Hence rename slices to something else or add a comment saying how many 16GB slices. Also explain why you are taking 16GB as the size. > +#define ADDR_MARK_128TB 0x800000000000UL /* Beyond 128TB */ > + > +static char *hind_addr(void) > +{ > + int bits = 48 + rand() % 15; > + > + return (char *) (1UL << bits); > +} > + > +static int validate_addr(char *ptr, int high_addr) > +{ > + unsigned long addr = (unsigned long) ptr; > + > + if (high_addr) { > + if (addr < ADDR_MARK_128TB) { > + printf("Bad address %lx\n", addr); > + return 1; > + } > + return 0; > + } > + > + if (addr > ADDR_MARK_128TB) { > + printf("Bad address %lx\n", addr); > + return 1; > + } > + return 0; > +} > + > +int vaddr(void) > +{ > + char *ptr[NR_SLICES_128TB]; > + char *hptr[NR_SLICES_384TB]; > + char *hint; > + int i; > + > + for (i = 0; i < NR_SLICES_128TB; i++) { > + ptr[i] = mmap(NULL, MAP_SIZE_16GB, PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + > + if (ptr[i] == MAP_FAILED) > + break; > + > + if (validate_addr(ptr[i], 0)) > + return 1; > + } > + > + for (i = 0; i < NR_SLICES_384TB; i++) { > + hint = hind_addr(); Why add rand() why not a constant value ? > + hptr[i] = mmap(hint, MAP_SIZE_16GB, PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + > + if (hptr[i] == MAP_FAILED) > + break; > + > + if (validate_addr(hptr[i], 1)) > + return 1; > + } We can also add check to make sure mmap fail if we passed a hint addr below 128TB ? > + > + for (i = 0; i < NR_SLICES_128TB; i++) > + munmap(ptr[i], MAP_SIZE_16GB); > + > + for (i = 0; i < NR_SLICES_384TB; i++) > + munmap(hptr[i], MAP_SIZE_16GB); > + return 0; > +} > + > +int main(int argc, char *argv[]) > +{ > + return test_harness(vaddr, "vaddr-range"); > +} > -- > 1.8.5.2
On 04/17/2017 10:56 AM, Aneesh Kumar K.V wrote: > Anshuman Khandual <khandual@linux.vnet.ibm.com> writes: > >> This verifies virtual address mapping below and above the >> 128TB range and makes sure that address returned are within >> the expected range depending upon the hint passed from the >> user space. >> >> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com> >> --- >> Tested this on latest ppc-next with Aneesh's yesterday's patch >> which can be found at https://patchwork.ozlabs.org/patch/749510/ >> >> tools/testing/selftests/powerpc/mm/.gitignore | 3 +- >> tools/testing/selftests/powerpc/mm/Makefile | 2 +- >> tools/testing/selftests/powerpc/mm/vaddr.c | 89 +++++++++++++++++++++++++++ >> 3 files changed, 92 insertions(+), 2 deletions(-) >> create mode 100644 tools/testing/selftests/powerpc/mm/vaddr.c >> >> diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore >> index e715a3f..af1f31f 100644 >> --- a/tools/testing/selftests/powerpc/mm/.gitignore >> +++ b/tools/testing/selftests/powerpc/mm/.gitignore >> @@ -1,4 +1,5 @@ >> hugetlb_vs_thp_test >> subpage_prot >> tempfile >> -prot_sao >> \ No newline at end of file >> +prot_sao >> +vaddr >> diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile >> index 1cffe54..7345db2 100644 >> --- a/tools/testing/selftests/powerpc/mm/Makefile >> +++ b/tools/testing/selftests/powerpc/mm/Makefile >> @@ -1,7 +1,7 @@ >> noarg: >> $(MAKE) -C ../ >> >> -TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao >> +TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao vaddr >> TEST_GEN_FILES := tempfile >> >> include ../../lib.mk >> diff --git a/tools/testing/selftests/powerpc/mm/vaddr.c b/tools/testing/selftests/powerpc/mm/vaddr.c >> new file mode 100644 >> index 0000000..d6485e9 >> --- /dev/null >> +++ b/tools/testing/selftests/powerpc/mm/vaddr.c >> @@ -0,0 +1,89 @@ >> +/* >> + * Copyright 2017, Anshuman Khandual, IBM Corp. >> + * Licensed under GPLv2. >> + */ >> + >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <string.h> >> +#include <unistd.h> >> +#include <errno.h> >> +#include <numaif.h> >> +#include <sys/mman.h> >> +#include <sys/time.h> >> +#include "utils.h" >> + >> +#define PAGE_SIZE 65536UL /* 64KB */ >> +#define MAP_SIZE_16GB 262144UL /* 16GB */ > > can we make it MMAP_CHUNK_SIZE ? Sure, will do. > >> +#define NR_SLICES_128TB 8192UL /* 128TB */ > > NR_CHUNK_128TB ? Sure, will do. > >> +#define NR_SLICES_384TB 24576UL /* 384TB */ > > Can you add more comments around that. Those numbers/names have special > meaning on powerpc. I guess you are not doing anything related that. > Hence rename slices to something else or add a comment saying how many > 16GB slices. Sure will rename as CHUNKS instead. > Also explain why you are taking 16GB as the size. Sure, will do. > >> +#define ADDR_MARK_128TB 0x800000000000UL /* Beyond 128TB */ >> + >> +static char *hind_addr(void) >> +{ >> + int bits = 48 + rand() % 15; >> + >> + return (char *) (1UL << bits); >> +} >> + >> +static int validate_addr(char *ptr, int high_addr) >> +{ >> + unsigned long addr = (unsigned long) ptr; >> + >> + if (high_addr) { >> + if (addr < ADDR_MARK_128TB) { >> + printf("Bad address %lx\n", addr); >> + return 1; >> + } >> + return 0; >> + } >> + >> + if (addr > ADDR_MARK_128TB) { >> + printf("Bad address %lx\n", addr); >> + return 1; >> + } >> + return 0; >> +} >> + >> +int vaddr(void) >> +{ >> + char *ptr[NR_SLICES_128TB]; >> + char *hptr[NR_SLICES_384TB]; >> + char *hint; >> + int i; >> + >> + for (i = 0; i < NR_SLICES_128TB; i++) { >> + ptr[i] = mmap(NULL, MAP_SIZE_16GB, PROT_READ | PROT_WRITE, >> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >> + >> + if (ptr[i] == MAP_FAILED) >> + break; >> + >> + if (validate_addr(ptr[i], 0)) >> + return 1; >> + } >> + >> + for (i = 0; i < NR_SLICES_384TB; i++) { >> + hint = hind_addr(); > > Why add rand() why not a constant value ? Because it can test hint address range starting from 128TB till 2^64. > >> + hptr[i] = mmap(hint, MAP_SIZE_16GB, PROT_READ | PROT_WRITE, >> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >> + >> + if (hptr[i] == MAP_FAILED) >> + break; >> + >> + if (validate_addr(hptr[i], 1)) >> + return 1; >> + } > > We can also add check to make sure mmap fail if we passed a hint addr > below 128TB ? After all of the range lower to the 128TB address is mapped ?
Anshuman Khandual <khandual@linux.vnet.ibm.com> writes: > On 04/15/2017 01:48 AM, Michal Suchanek wrote: >> Hello, >> >> On Wed, 12 Apr 2017 15:11:12 +0530 >> Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote: >> >>> This verifies virtual address mapping below and above the >>> 128TB range and makes sure that address returned are within >>> the expected range depending upon the hint passed from the >>> user space. >> >> This description does not mention anything PPC specific. Can this be >> generalized to work with Intel L5 enabled kernel as well? > > Hmm, I am not completely aware about the proposed mmap interface > for 128TB and beyond till 512TB by Kirill on x86. If both powerpc > and x86 will have the same semantics, then this test is generic > enough to cover both the architectures and will move it into > selftests/vm/ instead of current selftests/powerpc/mm. I think it can be made generic, maybe it needs a few #ifdefs, but that's fine for a selftest. So yeah if you can move it to selftests/vm I think that would be better. cheers
diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore index e715a3f..af1f31f 100644 --- a/tools/testing/selftests/powerpc/mm/.gitignore +++ b/tools/testing/selftests/powerpc/mm/.gitignore @@ -1,4 +1,5 @@ hugetlb_vs_thp_test subpage_prot tempfile -prot_sao \ No newline at end of file +prot_sao +vaddr diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile index 1cffe54..7345db2 100644 --- a/tools/testing/selftests/powerpc/mm/Makefile +++ b/tools/testing/selftests/powerpc/mm/Makefile @@ -1,7 +1,7 @@ noarg: $(MAKE) -C ../ -TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao +TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao vaddr TEST_GEN_FILES := tempfile include ../../lib.mk diff --git a/tools/testing/selftests/powerpc/mm/vaddr.c b/tools/testing/selftests/powerpc/mm/vaddr.c new file mode 100644 index 0000000..d6485e9 --- /dev/null +++ b/tools/testing/selftests/powerpc/mm/vaddr.c @@ -0,0 +1,89 @@ +/* + * Copyright 2017, Anshuman Khandual, IBM Corp. + * Licensed under GPLv2. + */ + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <errno.h> +#include <numaif.h> +#include <sys/mman.h> +#include <sys/time.h> +#include "utils.h" + +#define PAGE_SIZE 65536UL /* 64KB */ +#define MAP_SIZE_16GB 262144UL /* 16GB */ +#define NR_SLICES_128TB 8192UL /* 128TB */ +#define NR_SLICES_384TB 24576UL /* 384TB */ +#define ADDR_MARK_128TB 0x800000000000UL /* Beyond 128TB */ + +static char *hind_addr(void) +{ + int bits = 48 + rand() % 15; + + return (char *) (1UL << bits); +} + +static int validate_addr(char *ptr, int high_addr) +{ + unsigned long addr = (unsigned long) ptr; + + if (high_addr) { + if (addr < ADDR_MARK_128TB) { + printf("Bad address %lx\n", addr); + return 1; + } + return 0; + } + + if (addr > ADDR_MARK_128TB) { + printf("Bad address %lx\n", addr); + return 1; + } + return 0; +} + +int vaddr(void) +{ + char *ptr[NR_SLICES_128TB]; + char *hptr[NR_SLICES_384TB]; + char *hint; + int i; + + for (i = 0; i < NR_SLICES_128TB; i++) { + ptr[i] = mmap(NULL, MAP_SIZE_16GB, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + + if (ptr[i] == MAP_FAILED) + break; + + if (validate_addr(ptr[i], 0)) + return 1; + } + + for (i = 0; i < NR_SLICES_384TB; i++) { + hint = hind_addr(); + hptr[i] = mmap(hint, MAP_SIZE_16GB, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + + if (hptr[i] == MAP_FAILED) + break; + + if (validate_addr(hptr[i], 1)) + return 1; + } + + for (i = 0; i < NR_SLICES_128TB; i++) + munmap(ptr[i], MAP_SIZE_16GB); + + for (i = 0; i < NR_SLICES_384TB; i++) + munmap(hptr[i], MAP_SIZE_16GB); + return 0; +} + +int main(int argc, char *argv[]) +{ + return test_harness(vaddr, "vaddr-range"); +}
This verifies virtual address mapping below and above the 128TB range and makes sure that address returned are within the expected range depending upon the hint passed from the user space. Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com> --- Tested this on latest ppc-next with Aneesh's yesterday's patch which can be found at https://patchwork.ozlabs.org/patch/749510/ tools/testing/selftests/powerpc/mm/.gitignore | 3 +- tools/testing/selftests/powerpc/mm/Makefile | 2 +- tools/testing/selftests/powerpc/mm/vaddr.c | 89 +++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/powerpc/mm/vaddr.c