| Message ID | 20151207075133.32696.92131.stgit@dwillia2-desk3.jf.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Dec 6, 2015 at 11:51 PM, Dan Williams <dan.j.williams@intel.com> wrote: > From: Toshi Kani <toshi.kani@hpe.com> > > Permute through various parameters to mmap when establishing dax > mappings. Ideally we would have fuzz testing for this interface, but > for now this just does nominal testing of several options. > > All the test code is from Toshi, with integration into ndctl by Dan. > > Not-yet-signed-off-by: Toshi Kani <toshi.kani@hpe.com> Hi Toshi, can I add your sign-off for this implementation? Thanks for developing the test, it fleshed out a good handful of kernel bugs.
On Mon, 2015-12-07 at 10:43 -0800, Dan Williams wrote: > On Sun, Dec 6, 2015 at 11:51 PM, Dan Williams <dan.j.williams@intel.com> > wrote: > > From: Toshi Kani <toshi.kani@hpe.com> > > > > Permute through various parameters to mmap when establishing dax > > mappings. Ideally we would have fuzz testing for this interface, but > > for now this just does nominal testing of several options. > > > > All the test code is from Toshi, with integration into ndctl by Dan. > > > > Not-yet-signed-off-by: Toshi Kani <toshi.kani@hpe.com> > > Hi Toshi, can I add your sign-off for this implementation? Thanks for > developing the test, it fleshed out a good handful of kernel bugs. Yes, and thanks for integrating the test! Signed-off-by: Toshi Kani <toshi.kani@hpe.com> -Toshi
> -----Original Message----- > From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of > Dan Williams > Sent: Monday, December 7, 2015 1:52 AM > To: linux-nvdimm@lists.01.org > Cc: toshi.kani@hp.com > Subject: [PATCH] ndctl: general dax mmap test > > From: Toshi Kani <toshi.kani@hpe.com> > > Permute through various parameters to mmap when establishing dax > mappings. Ideally we would have fuzz testing for this interface, but > for now this just does nominal testing of several options. > > All the test code is from Toshi, with integration into ndctl by Dan. > > Not-yet-signed-off-by: Toshi Kani <toshi.kani@hpe.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> ... > diff --git a/lib/test-mmap.c b/lib/test-mmap.c > new file mode 100644 > index 000000000000..d0134fa44712 > --- /dev/null > +++ b/lib/test-mmap.c > @@ -0,0 +1,220 @@ ... > +#define MB(a) ((a) * 1024UL * 1024UL) I suggest using MiB there. ... > +static void test_write(unsigned long *p, size_t size) > +{ > + size_t i; > + unsigned long *wp; > + unsigned long long timeval; > + > + start(); > + for (i=0, wp=p; i<(size/sizeof(wp)); i++) > + *wp++ = 1; I think that should be sizeof(*wp), the size of unsigned long, not the size of the pointer to the unsigned long. > + timeval = stop(); > + printf("Write: %10llu usec\n", timeval); > +} > + > +static void test_read(unsigned long *p, size_t size) > +{ > + size_t i; > + volatile unsigned long *wp, tmp; > + unsigned long long timeval; > + > + start(); > + for (i=0, wp=p; i<(size/sizeof(wp)); i++) > + tmp = *wp++; sizeof(*wp) like above. > + tmp = tmp; That seems like a NOP, unless it's relying on some special compiler handling of volatile. > + timeval = stop(); > + printf("Read : %10llu usec\n", timeval); > +} > + > +int main(int argc, char **argv) ... > + case 'S': > + printf("> mlock - skip first ite\n"); I would spell out iteration. ... > + if ((long unsigned)p & (MB(2)-1)) > + printf("> mmap: NOT 2MB aligned: 0x%p\n", p); > + else > + printf("> mmap: 2MB aligned: 0x%p\n", p); I suggest using MiB rather than MB in the prints. > + > +#if 0 /* SIZE LIMIT */ > + if (size >= MB(2)) > + cpy_size = MB(32); > + else > +#endif > + cpy_size = size; Any real need for that if 0 code? ... > diff --git a/lib/test-mmap.sh b/lib/test-mmap.sh > new file mode 100755 > index 000000000000..8d024e57925d > --- /dev/null > +++ b/lib/test-mmap.sh ... > + $TEST -rwps $MNT/$FILE # popluate, shared (popluate no effect) > + $TEST -Rrps $MNT/$FILE # read-only popluate, shared (popluate popluate should be populate. --- Robert Elliott, HPE Persistent Memory
On Mon, 2015-12-07 at 19:32 +0000, Elliott, Robert (Persistent Memory) wrote: > > -----Original Message----- > > From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf > > Of > > Dan Williams > > Sent: Monday, December 7, 2015 1:52 AM > > To: linux-nvdimm@lists.01.org > > Cc: toshi.kani@hp.com > > Subject: [PATCH] ndctl: general dax mmap test > > > > From: Toshi Kani <toshi.kani@hpe.com> > > > > Permute through various parameters to mmap when establishing dax > > mappings. Ideally we would have fuzz testing for this interface, but > > for now this just does nominal testing of several options. > > > > All the test code is from Toshi, with integration into ndctl by Dan. > > > > Not-yet-signed-off-by: Toshi Kani <toshi.kani@hpe.com> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > ... > > diff --git a/lib/test-mmap.c b/lib/test-mmap.c > > new file mode 100644 > > index 000000000000..d0134fa44712 > > --- /dev/null > > +++ b/lib/test-mmap.c > > @@ -0,0 +1,220 @@ > ... > > +#define MB(a) ((a) * 1024UL * 1024UL) > > I suggest using MiB there. > > ... > > +static void test_write(unsigned long *p, size_t size) > > +{ > > + size_t i; > > + unsigned long *wp; > > + unsigned long long timeval; > > + > > + start(); > > + for (i=0, wp=p; i<(size/sizeof(wp)); i++) > > + *wp++ = 1; > > I think that should be sizeof(*wp), the size of > unsigned long, not the size of the pointer to the > unsigned long. Good catch. > > + timeval = stop(); > > + printf("Write: %10llu usec\n", timeval); > > +} > > + > > +static void test_read(unsigned long *p, size_t size) > > +{ > > + size_t i; > > + volatile unsigned long *wp, tmp; > > + unsigned long long timeval; > > + > > + start(); > > + for (i=0, wp=p; i<(size/sizeof(wp)); i++) > > + tmp = *wp++; > > sizeof(*wp) like above. Right. > > + tmp = tmp; > > That seems like a NOP, unless it's relying on some special > compiler handling of volatile. > > > + timeval = stop(); > > + printf("Read : %10llu usec\n", timeval); > > +} > > + > > +int main(int argc, char **argv) > ... > > + case 'S': > > + printf("> mlock - skip first ite\n"); > > I would spell out iteration. > > ... > > + if ((long unsigned)p & (MB(2)-1)) > > + printf("> mmap: NOT 2MB aligned: 0x%p\n", p); > > + else > > + printf("> mmap: 2MB aligned: 0x%p\n", p); > > I suggest using MiB rather than MB in the prints. > > > + > > +#if 0 /* SIZE LIMIT */ > > + if (size >= MB(2)) > > + cpy_size = MB(32); > > + else > > +#endif > > + cpy_size = size; > > Any real need for that if 0 code? No, this is a dead code (and the numbers do not even match..) and can be removed. > ... > > diff --git a/lib/test-mmap.sh b/lib/test-mmap.sh > > new file mode 100755 > > index 000000000000..8d024e57925d > > --- /dev/null > > +++ b/lib/test-mmap.sh > ... > > + $TEST -rwps $MNT/$FILE # popluate, shared (popluate no > > effect) > > + $TEST -Rrps $MNT/$FILE # read-only popluate, shared > > (popluate > > popluate should be populate. Dan, would you mind to update the patch in your branch? (Let me know if you'd need me to update.) Thanks, -Toshi
diff --git a/Makefile.am b/Makefile.am index 2989e076972d..7fee9c688c84 100644 --- a/Makefile.am +++ b/Makefile.am @@ -123,8 +123,8 @@ if ENABLE_DESTRUCTIVE TESTS += lib/test-blk-ns lib/test-pmem-ns lib/test-pcommit check_PROGRAMS += lib/test-blk-ns lib/test-pmem-ns lib/test-pcommit -TESTS += lib/test-dax-dev lib/test-dax.sh -check_PROGRAMS += lib/test-dax-dev lib/test-dax-pmd +TESTS += lib/test-dax-dev lib/test-dax.sh lib/test-mmap.sh +check_PROGRAMS += lib/test-dax-dev lib/test-dax-pmd lib/test-mmap endif lib_test_libndctl_SOURCES = lib/test-libndctl.c lib/test-core.c @@ -149,3 +149,4 @@ lib_test_dax_dev_SOURCES = lib/test-dax-dev.c lib/test-core.c lib_test_dax_dev_LDADD = lib/libndctl.la lib_test_dax_pmd_SOURCES = lib/test-dax-pmd.c +lib_test_mmap_SOURCES = lib/test-mmap.c diff --git a/lib/test-mmap.c b/lib/test-mmap.c new file mode 100644 index 000000000000..d0134fa44712 --- /dev/null +++ b/lib/test-mmap.c @@ -0,0 +1,220 @@ +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/mman.h> +#include <sys/time.h> +#include <string.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#define MB(a) ((a) * 1024UL * 1024UL) + +static struct timeval start_tv, stop_tv; + +// Calculate the difference between two time values. +static void tvsub(struct timeval *tdiff, struct timeval *t1, struct timeval *t0) +{ + tdiff->tv_sec = t1->tv_sec - t0->tv_sec; + tdiff->tv_usec = t1->tv_usec - t0->tv_usec; + if (tdiff->tv_usec < 0) + tdiff->tv_sec--, tdiff->tv_usec += 1000000; +} + +// Start timing now. +static void start(void) +{ + (void) gettimeofday(&start_tv, (struct timezone *) 0); +} + +// Stop timing and return real time in microseconds. +static unsigned long long stop(void) +{ + struct timeval tdiff; + + (void) gettimeofday(&stop_tv, (struct timezone *) 0); + tvsub(&tdiff, &stop_tv, &start_tv); + return (tdiff.tv_sec * 1000000 + tdiff.tv_usec); +} + +static void test_write(unsigned long *p, size_t size) +{ + size_t i; + unsigned long *wp; + unsigned long long timeval; + + start(); + for (i=0, wp=p; i<(size/sizeof(wp)); i++) + *wp++ = 1; + timeval = stop(); + printf("Write: %10llu usec\n", timeval); +} + +static void test_read(unsigned long *p, size_t size) +{ + size_t i; + volatile unsigned long *wp, tmp; + unsigned long long timeval; + + start(); + for (i=0, wp=p; i<(size/sizeof(wp)); i++) + tmp = *wp++; + tmp = tmp; + timeval = stop(); + printf("Read : %10llu usec\n", timeval); +} + +int main(int argc, char **argv) +{ + int fd, i, opt, ret; + int oflags, mprot, mflags = 0; + int is_read_only = 0, is_mlock = 0, is_mlockall = 0; + int mlock_skip = 0, read_test = 0, write_test = 0; + void *mptr = NULL; + unsigned long *p; + struct stat stat; + size_t size, cpy_size; + const char *file_name = NULL; + + while ((opt = getopt(argc, argv, "RMSApsrw")) != -1) { + switch (opt) { + case 'R': + printf("> mmap: read-only\n"); + is_read_only = 1; + break; + case 'M': + printf("> mlock\n"); + is_mlock = 1; + break; + case 'S': + printf("> mlock - skip first ite\n"); + mlock_skip = 1; + break; + case 'A': + printf("> mlockall\n"); + is_mlockall = 1; + break; + case 'p': + printf("> MAP_POPULATE\n"); + mflags |= MAP_POPULATE; + break; + case 's': + printf("> MAP_SHARED\n"); + mflags |= MAP_SHARED; + break; + case 'r': + printf("> read-test\n"); + read_test = 1; + break; + case 'w': + printf("> write-test\n"); + write_test = 1; + break; + } + } + + if (optind == argc) { + printf("missing file name\n"); + return EXIT_FAILURE; + } + file_name = argv[optind]; + + if (!(mflags & MAP_SHARED)) { + printf("> MAP_PRIVATE\n"); + mflags |= MAP_PRIVATE; + } + + if (is_read_only) { + oflags = O_RDONLY; + mprot = PROT_READ; + } else { + oflags = O_RDWR; + mprot = PROT_READ|PROT_WRITE; + } + + fd = open(file_name, oflags); + if (fd == -1) { + perror("open failed"); + return EXIT_FAILURE; + } + + ret = fstat(fd, &stat); + if (ret < 0) { + perror("fstat failed"); + return EXIT_FAILURE; + } + size = stat.st_size; + + printf("> open %s size %#zx flags %#x\n", file_name, size, oflags); + + ret = posix_memalign(&mptr, MB(2), size); + if (ret ==0) + free(mptr); + + printf("> mmap mprot 0x%x flags 0x%x\n", mprot, mflags); + p = mmap(mptr, size, mprot, mflags, fd, 0x0); + if (!p) { + perror("mmap failed"); + return EXIT_FAILURE; + } + if ((long unsigned)p & (MB(2)-1)) + printf("> mmap: NOT 2MB aligned: 0x%p\n", p); + else + printf("> mmap: 2MB aligned: 0x%p\n", p); + +#if 0 /* SIZE LIMIT */ + if (size >= MB(2)) + cpy_size = MB(32); + else +#endif + cpy_size = size; + + for (i=0; i<3; i++) { + + if (is_mlock && !mlock_skip) { + printf("> mlock 0x%p\n", p); + ret = mlock(p, size); + if (ret < 0) { + perror("mlock failed"); + return EXIT_FAILURE; + } + } else if (is_mlockall) { + printf("> mlockall\n"); + ret = mlockall(MCL_CURRENT|MCL_FUTURE); + if (ret < 0) { + perror("mlockall failed"); + return EXIT_FAILURE; + } + } + + printf("===== %d =====\n", i+1); + if (write_test) + test_write(p, cpy_size); + if (read_test) + test_read(p, cpy_size); + + if (is_mlock && !mlock_skip) { + printf("> munlock 0x%p\n", p); + ret = munlock(p, size); + if (ret < 0) { + perror("munlock failed"); + return EXIT_FAILURE; + } + } else if (is_mlockall) { + printf("> munlockall\n"); + ret = munlockall(); + if (ret < 0) { + perror("munlockall failed"); + return EXIT_FAILURE; + } + } + + /* skip, if requested, only the first iteration */ + mlock_skip = 0; + } + + printf("> munmap 0x%p\n", p); + munmap(p, size); + return EXIT_SUCCESS; +} + diff --git a/lib/test-mmap.sh b/lib/test-mmap.sh new file mode 100755 index 000000000000..8d024e57925d --- /dev/null +++ b/lib/test-mmap.sh @@ -0,0 +1,61 @@ +#!/bin/bash +MNT=test_mmap_mnt +FILE=image +DEV="" +TEST=lib/test-mmap + +err() { + rc=1 + echo "test-mmap: failed at line $1" + if [ -n "$DEV" ]; then + umount $DEV + else + rc=77 + fi + rmdir $MNT + exit $rc +} + +test_mmap() { + trap 'err $LINENO' ERR + + # SHARED + $TEST -Mrwps $MNT/$FILE # mlock, populate, shared (mlock fail) + $TEST -Arwps $MNT/$FILE # mlockall, populate, shared + $TEST -RMrps $MNT/$FILE # read-only, mlock, populate, shared (mlock fail) + $TEST -rwps $MNT/$FILE # popluate, shared (popluate no effect) + $TEST -Rrps $MNT/$FILE # read-only popluate, shared (popluate no effect) + $TEST -Mrws $MNT/$FILE # mlock, shared (mlock fail) + $TEST -RMrs $MNT/$FILE # read-only, mlock, shared (mlock fail) + $TEST -rws $MNT/$FILE # shared (ok) + $TEST -Rrs $MNT/$FILE # read-only, shared (ok) + + # PRIVATE + $TEST -Mrwp $MNT/$FILE # mlock, populate, private (ok) + $TEST -RMrp $MNT/$FILE # read-only, mlock, populate, private (mlock fail) + $TEST -rwp $MNT/$FILE # populate, private (ok) + $TEST -Rrp $MNT/$FILE # read-only, populate, private (populate no effect) + $TEST -Mrw $MNT/$FILE # mlock, private (ok) + $TEST -RMr $MNT/$FILE # read-only, mlock, private (mlock fail) + $TEST -MSr $MNT/$FILE # private, read before mlock (ok) + $TEST -rw $MNT/$FILE # private (ok) + $TEST -Rr $MNT/$FILE # read-only, private (ok) +} + +set -e +mkdir -p $MNT +trap 'err $LINENO' ERR + +DEV=$(lib/test-dax-dev) + +mkfs.ext4 $DEV +mount $DEV $MNT -o dax +fallocate -l 1GiB $MNT/$FILE +test_mmap +umount $MNT + +mkfs.xfs -f $DEV +mount $DEV $MNT -o dax +fallocate -l 1GiB $MNT/$FILE +test_mmap +umount $MNT