| Message ID | 1489752862-15786-1-git-send-email-zlang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 17, 2017 at 2:14 PM, Zorro Lang <zlang@redhat.com> wrote: > mmap as a popular and basic operation, most of softwares use it to > access files. > > More and more customers report bugs related with mmap/munmap and > other stress conditions. So add mmap test into fsstress to reproduce > or find more bugs easily. > > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > > Hi, > > I've tested this patch on XFS by running: > > ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f creat=500 -f mkdir=500 -n 2000 -p10 -v > ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f dwrite=1000 -f write=1000 -f creat=500 -f mkdir=500 -n 4000 -p10 -v > ./ltp/fsstress -d /mnt/scratch -n 5000 -p10 -v > > I didn't find any unexpected errors. Please help to review. > I'll send another patch to LTP if this patch can be merged into xfstests. > > Thanks, > Zorro > > ltp/fsstress.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/global.h | 4 +++ > 2 files changed, 81 insertions(+) > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > index 35e2765..5914551 100644 > --- a/ltp/fsstress.c > +++ b/ltp/fsstress.c > @@ -2585,7 +2585,84 @@ link_f(int opno, long r) > void > mmap_f(int opno, long r) > { IMO, the operation you implemented would be better named 'mwrite' and it makes sense while you're at it to add 'mread' counterpart. It might have been nice to have 'mmap'/'munmap' commands which actually maintain a set of files on which mwrite/mread acts upon, similarly to the relation between creat/unlink and the rest of the file ops. This could actually allow to stress test concurrent access the shared memory between processes, but it is going to be a lot more work... So for now mwrite/mread imply mmap+mwrite/mread+munmap Amir. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 17, 2017 at 02:44:13PM +0200, Amir Goldstein wrote: > On Fri, Mar 17, 2017 at 2:14 PM, Zorro Lang <zlang@redhat.com> wrote: > > mmap as a popular and basic operation, most of softwares use it to > > access files. > > > > More and more customers report bugs related with mmap/munmap and > > other stress conditions. So add mmap test into fsstress to reproduce > > or find more bugs easily. > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > --- > > > > Hi, > > > > I've tested this patch on XFS by running: > > > > ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f creat=500 -f mkdir=500 -n 2000 -p10 -v > > ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f dwrite=1000 -f write=1000 -f creat=500 -f mkdir=500 -n 4000 -p10 -v > > ./ltp/fsstress -d /mnt/scratch -n 5000 -p10 -v > > > > I didn't find any unexpected errors. Please help to review. > > I'll send another patch to LTP if this patch can be merged into xfstests. > > > > Thanks, > > Zorro > > > > ltp/fsstress.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > src/global.h | 4 +++ > > 2 files changed, 81 insertions(+) > > > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > > index 35e2765..5914551 100644 > > --- a/ltp/fsstress.c > > +++ b/ltp/fsstress.c > > @@ -2585,7 +2585,84 @@ link_f(int opno, long r) > > void > > mmap_f(int opno, long r) > > { Hi Amir, Thanks for your reviewing. > > IMO, the operation you implemented would be better named 'mwrite' > and it makes sense while you're at it to add 'mread' counterpart. Hmm, make sense. > > It might have been nice to have 'mmap'/'munmap' commands > which actually maintain a set of files on which mwrite/mread acts > upon, similarly to the relation between creat/unlink and the rest of > the file ops. > > This could actually allow to stress test concurrent access the shared memory > between processes, but it is going to be a lot more work... If so, I have to record addr and length with file together in mwrite when mmap a file, and one file maybe mapped not only once. Then mread need to choose one addr+length+file group randomly to test and munmap. Hmm... That's a good suggestion. But I just hope to add some mmap/munmap random load I/Os in fsstress, to trigger more writeback + munmap + memory reclaim race situations in this simple patch. I didn't plan to test concurrent access the shared memory this time, due to what you said, it's going to be lots more work:) How about add this feature in later patch, after this patch can be merged? What do you think? Thanks, Zorro > So for now mwrite/mread imply mmap+mwrite/mread+munmap > > Amir. > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 17, 2017 at 3:27 PM, Zorro Lang <zlang@redhat.com> wrote: > On Fri, Mar 17, 2017 at 02:44:13PM +0200, Amir Goldstein wrote: >> On Fri, Mar 17, 2017 at 2:14 PM, Zorro Lang <zlang@redhat.com> wrote: >> > mmap as a popular and basic operation, most of softwares use it to >> > access files. >> > >> > More and more customers report bugs related with mmap/munmap and >> > other stress conditions. So add mmap test into fsstress to reproduce >> > or find more bugs easily. >> > >> > Signed-off-by: Zorro Lang <zlang@redhat.com> >> > --- >> > >> > Hi, >> > >> > I've tested this patch on XFS by running: >> > >> > ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f creat=500 -f mkdir=500 -n 2000 -p10 -v >> > ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f dwrite=1000 -f write=1000 -f creat=500 -f mkdir=500 -n 4000 -p10 -v >> > ./ltp/fsstress -d /mnt/scratch -n 5000 -p10 -v >> > >> > I didn't find any unexpected errors. Please help to review. >> > I'll send another patch to LTP if this patch can be merged into xfstests. >> > >> > Thanks, >> > Zorro >> > >> > ltp/fsstress.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> > src/global.h | 4 +++ >> > 2 files changed, 81 insertions(+) >> > >> > diff --git a/ltp/fsstress.c b/ltp/fsstress.c >> > index 35e2765..5914551 100644 >> > --- a/ltp/fsstress.c >> > +++ b/ltp/fsstress.c >> > @@ -2585,7 +2585,84 @@ link_f(int opno, long r) >> > void >> > mmap_f(int opno, long r) >> > { > > Hi Amir, > > Thanks for your reviewing. > >> >> IMO, the operation you implemented would be better named 'mwrite' >> and it makes sense while you're at it to add 'mread' counterpart. > > Hmm, make sense. > >> >> It might have been nice to have 'mmap'/'munmap' commands >> which actually maintain a set of files on which mwrite/mread acts >> upon, similarly to the relation between creat/unlink and the rest of >> the file ops. >> >> This could actually allow to stress test concurrent access the shared memory >> between processes, but it is going to be a lot more work... > > If so, I have to record addr and length with file together in mwrite when > mmap a file, and one file maybe mapped not only once. Then mread > need to choose one addr+length+file group randomly to test and munmap. > > Hmm... That's a good suggestion. But I just hope to add some mmap/munmap > random load I/Os in fsstress, to trigger more writeback + munmap + memory reclaim > race situations in this simple patch. I didn't plan to test concurrent access > the shared memory this time, due to what you said, it's going to be lots more > work:) > > How about add this feature in later patch, after this patch can be merged? > What do you think? > Certainly. I wasn't implying that your patch should be blocked. My only review comment was to name of the operation which is misleading. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/ltp/fsstress.c b/ltp/fsstress.c index 35e2765..5914551 100644 --- a/ltp/fsstress.c +++ b/ltp/fsstress.c @@ -2585,7 +2585,84 @@ link_f(int opno, long r) void mmap_f(int opno, long r) { + char *addr; + int e; + pathname_t f; + int fd; + size_t len; + __int64_t lr; + off64_t off; + int flags; + struct stat64 stb; + int v; + char st[1024]; + init_pathname(&f); + if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) { + if (v) + printf("%d/%d: mmap - no filename\n", procid, opno); + free_pathname(&f); + return; + } + fd = open_path(&f, O_RDWR); + e = fd < 0 ? errno : 0; + check_cwd(); + if (fd < 0) { + if (v) + printf("%d/%d: mmap - open %s failed %d\n", + procid, opno, f.path, e); + free_pathname(&f); + return; + } + if (fstat64(fd, &stb) < 0) { + if (v) + printf("%d/%d: write - fstat64 %s failed %d\n", + procid, opno, f.path, errno); + free_pathname(&f); + close(fd); + return; + } + inode_info(st, sizeof(st), &stb, v); + lr = ((__int64_t)random() << 32) + random(); + off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE)); + off %= maxfsize; + off &= (off64_t)(~(sysconf(_SC_PAGE_SIZE) - 1)); + len = (size_t)(random() % MIN(maxfsize - off, FILELEN_MAX)) + 1; + + /* + * truncate file to the size we need to map and access, + * keep away SIGBUS / SIGSEGV killing this process + */ + e = truncate64_path(&f, off + len) < 0 ? errno : 0; + /* try private file mappings with 20% rate */ + flags = (random() % 20) ? MAP_SHARED : MAP_PRIVATE; + do { + addr = mmap(NULL, len, PROT_READ | PROT_WRITE, flags, fd, off); + e = (addr == MAP_FAILED) ? errno : 0; + if (errno == ENOMEM && flags & MAP_PRIVATE) { + /* turn to shared mapping if memeory is not enough for private mapping */ + flags = MAP_SHARED; + } else if (errno == ENOMEM && len > sysconf(_SC_PAGE_SIZE)) { + /* reduce mapping length, if memeory is not enough for shared mapping */ + len /= 2; + } + } while (errno == ENOMEM && len > sysconf(_SC_PAGE_SIZE)); + if (v) + printf("%d/%d: mmap %s%s [%lld,%d,%s] %d\n", + procid, opno, f.path, st, (long long)off, (int)len, + (flags & MAP_PRIVATE) ? "MAP_PRIVATE" : "MAP_SHARED", e); + + if (addr != MAP_FAILED) { + memset(addr, nameseq & 0xff, len); + e = munmap(addr, len) < 0 ? errno : 0; + if (v) + printf("%d/%d: munmap %s%s [%lld,%d] %d\n", + procid, opno, f.path, st, (long long)off, + (int)len, e); + } + + free_pathname(&f); + close(fd); } void diff --git a/src/global.h b/src/global.h index f63246b..51d1e94 100644 --- a/src/global.h +++ b/src/global.h @@ -178,4 +178,8 @@ #endif /* HAVE_LINUX_FALLOC_H */ +#ifndef HAVE_SYS_MMAN_H +#include <sys/mman.h> +#endif + #endif /* GLOBAL_H */
mmap as a popular and basic operation, most of softwares use it to access files. More and more customers report bugs related with mmap/munmap and other stress conditions. So add mmap test into fsstress to reproduce or find more bugs easily. Signed-off-by: Zorro Lang <zlang@redhat.com> --- Hi, I've tested this patch on XFS by running: ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f creat=500 -f mkdir=500 -n 2000 -p10 -v ./ltp/fsstress -d /mnt/scratch -f mmap=1000 -f dwrite=1000 -f write=1000 -f creat=500 -f mkdir=500 -n 4000 -p10 -v ./ltp/fsstress -d /mnt/scratch -n 5000 -p10 -v I didn't find any unexpected errors. Please help to review. I'll send another patch to LTP if this patch can be merged into xfstests. Thanks, Zorro ltp/fsstress.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/global.h | 4 +++ 2 files changed, 81 insertions(+)