On Wed, Mar 16, 2016 at 11:12:23PM +0800, Zorro Lang wrote:
> On Wed, Mar 16, 2016 at 10:22:53AM -0400, Brian Foster wrote:
> > On Wed, Mar 16, 2016 at 09:53:43PM +0800, Zorro Lang wrote:
> > > On Wed, Mar 16, 2016 at 08:31:14AM -0400, Brian Foster wrote:
> > > > On Wed, Mar 16, 2016 at 05:18:04PM +0800, Zorro Lang wrote:
> > > > > This patch come from a test likes below:
> > > > > xfs_io -t -f
> > > > > -c "truncate 10000"
> > > > > -c "mmap -rw 0 1024"
> > > > > -c "mremap 8192"
> > > > > file
> > > > >
> > > > > mremap always hit ENOMEM error, when it try to remap more space
> > > > > without MREMAP_MAYMOVE flag. This's a normal condition, due to
> > > > > no free space after mapped 1024 bytes region.
> > > > >
> > > > > But if we try to mremap from the original mapped starting point in
> > > > > a C program, at first we always do:
> > > > >
> > > > > addr = mmap(NULL, res_size, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > > munmap(addr, res_size);
> > > > >
> > > > > Then do:
> > > > >
> > > > > addr = mmap(addr, real_len, ...);
> > > > >
> > > > > The "res_size" is bigger than "real_len". This will help us get a
> > > > > region between real_len and res_size, which maybe free space. The
> > > > > truth is we can't guarantee that this free memory will stay free.
> > > > > But this method is still very helpful for reserve some free space
> > > > > in short time.
> > > > >
> > > > > After merge this patch, we can resolve above mremap problem by run:
> > > > > xfs_io -t -f
> > > > > ...
> > > > > -c "mmap -rw -s 8192 0 1024"
> > > > > -c "mremap 8192"
> > > > > ...
> > > > >
> > > > > Although we can't sure it's useful 100%, it really have pretty high
> > > > > success rate.
> > > > >
> > > > > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> > > > > ---
> > > >
> > > > I'm a little curious why one would use this as opposed to 'mremap -m' in
> > > > the context of xfs_io (it certainly might make sense for an
> > > > application). It sounds like any xfstests tests, for example, that is
> > > > susceptible to this problem might want to use -m even if -s is employed
> > > > as well. Can you provide any additional context on this or do you have a
> > > > use case in mind?
> > > >
> > > > That said, I'm not against adding this to the xfs_io toolbox and the
> > > > code looks Ok to me:
> > >
> > > Hi Brian,
> > >
> > > I hit this problem when I try to use xfs_io to rewrite LTP's mmap16
> > > testcase for xfstests(I know I can backport that mmap16.c into
> > > xfstests/src directly, but I just try to use xfs_io). That case
> > > from linux commit:
> > >
> > > 90a8020 vfs: fix data corruption when blocksize < pagesize for
> > > mmaped data
> > >
> > > For reproduce that bug, I think I shouldn't move page when mremap. So
> > > I need mremap to extend mapped space from orignal starting address.
> > >
> > > If no this option, xfs_io -c "mmap 0 1024" -c "mremap 8192" will
> > > hit ENOMEM error nearly 100%, but if use this option, it nearly always
> > > remap successfully(on my test machine). So I think it maybe helpful
> > > in the future, if someone try to mremap without "-m" :)
> > >
> >
> > Sounds good, thanks for the explanation! I might suggest to include this
> > use case in the commit log description for future reference (perhaps
> > condense it a bit first).
>
> Hi Brain,
>
> How about below commit message? If you feel it's OK, I will send a V2
> patch with this commit:
>
Looks fine to me, thanks!
Brian
> io/mmap: new -s option for mmap command to reserve some free space
>
> When I try to use xfs_io to rewrite LTP's mmap16 testcase for
> xfstests,
> I always hit mremap ENOMEM error. But according to linux commit:
>
> 90a8020 vfs: fix data corruption when blocksize < pagesize for
> mmaped data
>
> The reproducer shouldn't use MREMAP_MAYMOVE flag for mremap(). So we
> need a stable method to make mremap can extend space from the original
> mapped starting address(That means no -m option for xfs_io mremap).
>
> Generally if we try to mremap from the original mapped starting point
> in a C program, at first we always do:
>
> addr = mmap(NULL, res_size, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1,
> 0);
> munmap(addr, res_size);
>
> Then do:
>
> addr = mmap(addr, real_len, ...);
>
> The "res_size" is bigger than "real_len". This will help us get a
> region between real_len and res_size, which maybe free space. The
> truth is we can't guarantee that this free memory will stay free.
> But this method is still very helpful for reserve some free space
> in short time.
>
> This patch bring in the "res_size" by use a new -s option. If don't
> use
> this option, xfs_io -c "mmap 0 1024" -c "mremap 8192" will hit ENOMEM
> error nearly 100%, but if use this option, it nearly always remap
> successfully. So I think it's helpful, if someone try to run mremap
> without "-m".
>
> Thanks,
> Zorro
>
> >
> > Brian
> >
> > > Thanks,
> > > Zorro
> > >
> > > >
> > > > Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> > > >
> > > > > io/mmap.c | 29 +++++++++++++++++++++++------
> > > > > man/man8/xfs_io.8 | 17 ++++++++++++++++-
> > > > > 2 files changed, 39 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/io/mmap.c b/io/mmap.c
> > > > > index 5970069..6cd37a9 100644
> > > > > --- a/io/mmap.c
> > > > > +++ b/io/mmap.c
> > > > > @@ -146,6 +146,7 @@ mmap_help(void)
> > > > > " -r -- map with PROT_READ protection\n"
> > > > > " -w -- map with PROT_WRITE protection\n"
> > > > > " -x -- map with PROT_EXEC protection\n"
> > > > > +" -s <size> -- first do mmap(size)/munmap(size), try to reserve some free space\n"
> > > > > " If no protection mode is specified, all are used by default.\n"
> > > > > "\n"));
> > > > > }
> > > > > @@ -156,8 +157,8 @@ mmap_f(
> > > > > char **argv)
> > > > > {
> > > > > off64_t offset;
> > > > > - ssize_t length;
> > > > > - void *address;
> > > > > + ssize_t length = 0, length2 = 0;
> > > > > + void *address = NULL;
> > > > > char *filename;
> > > > > size_t blocksize, sectsize;
> > > > > int c, prot = 0;
> > > > > @@ -181,7 +182,9 @@ mmap_f(
> > > > > return 0;
> > > > > }
> > > > >
> > > > > - while ((c = getopt(argc, argv, "rwx")) != EOF) {
> > > > > + init_cvtnum(&blocksize, §size);
> > > > > +
> > > > > + while ((c = getopt(argc, argv, "rwxs:")) != EOF) {
> > > > > switch (c) {
> > > > > case 'r':
> > > > > prot |= PROT_READ;
> > > > > @@ -192,6 +195,9 @@ mmap_f(
> > > > > case 'x':
> > > > > prot |= PROT_EXEC;
> > > > > break;
> > > > > + case 's':
> > > > > + length2 = cvtnum(blocksize, sectsize, optarg);
> > > > > + break;
> > > > > default:
> > > > > return command_usage(&mmap_cmd);
> > > > > }
> > > > > @@ -202,7 +208,6 @@ mmap_f(
> > > > > if (optind != argc - 2)
> > > > > return command_usage(&mmap_cmd);
> > > > >
> > > > > - init_cvtnum(&blocksize, §size);
> > > > > offset = cvtnum(blocksize, sectsize, argv[optind]);
> > > > > if (offset < 0) {
> > > > > printf(_("non-numeric offset argument -- %s\n"), argv[optind]);
> > > > > @@ -221,7 +226,19 @@ mmap_f(
> > > > > return 0;
> > > > > }
> > > > >
> > > > > - address = mmap(NULL, length, prot, MAP_SHARED, file->fd, offset);
> > > > > + /*
> > > > > + * mmap and munmap memory area of length2 region is helpful to
> > > > > + * make a region of extendible free memory. It's generally used
> > > > > + * for later mremap operation(no MREMAP_MAYMOVE flag). But there
> > > > > + * isn't guarantee that the memory after length (up to length2)
> > > > > + * will stay free.
> > > > > + */
> > > > > + if (length2 > length) {
> > > > > + address = mmap(NULL, length2, prot,
> > > > > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > > + munmap(address, length2);
> > > > > + }
> > > > > + address = mmap(address, length, prot, MAP_SHARED, file->fd, offset);
> > > > > if (address == MAP_FAILED) {
> > > > > perror("mmap");
> > > > > free(filename);
> > > > > @@ -647,7 +664,7 @@ mmap_init(void)
> > > > > mmap_cmd.argmin = 0;
> > > > > mmap_cmd.argmax = -1;
> > > > > mmap_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK;
> > > > > - mmap_cmd.args = _("[N] | [-rwx] [off len]");
> > > > > + mmap_cmd.args = _("[N] | [-rwx] [-s size] [off len]");
> > > > > mmap_cmd.oneline =
> > > > > _("mmap a range in the current file, show mappings");
> > > > > mmap_cmd.help = mmap_help;
> > > > > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> > > > > index 33fbe6a..93a8a00 100644
> > > > > --- a/man/man8/xfs_io.8
> > > > > +++ b/man/man8/xfs_io.8
> > > > > @@ -559,7 +559,7 @@ Do not print timing statistics at all.
> > > > >
> > > > > .SH MEMORY MAPPED I/O COMMANDS
> > > > > .TP
> > > > > -.BI "mmap [ " N " | [[ \-rwx ] " "offset length " ]]
> > > > > +.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
> > > > > With no arguments,
> > > > > .B mmap
> > > > > shows the current mappings. Specifying a single numeric argument
> > > > > @@ -575,6 +575,21 @@ PROT_WRITE
> > > > > .RB ( \-w ),
> > > > > and PROT_EXEC
> > > > > .RB ( \-x ).
> > > > > +.BI \-s " size"
> > > > > +is used to do a mmap(size) && munmap(size) operation at first, try to reserve some
> > > > > +extendible free memory space, if
> > > > > +.I size
> > > > > +is bigger than
> > > > > +.I length
> > > > > +parameter. But there's not guarantee that the memory after
> > > > > +.I length
> > > > > +( up to
> > > > > +.I size
> > > > > +) will stay free.
> > > > > +.B e.g.
> > > > > +"mmap -rw -s 8192 1024" will mmap 0 ~ 1024 bytes memory, but try to reserve 1024 ~ 8192
> > > > > +free space(no guarantee). This free space will helpful for "mremap 8192" without
> > > > > +MREMAP_MAYMOVE flag.
> > > > > .TP
> > > > > .B mm
> > > > > See the
> > > >
> > > > _______________________________________________
> > > > xfs mailing list
> > > > xfs@xxxxxxxxxxx
> > > > http://oss.sgi.com/mailman/listinfo/xfs
> > >
> > > _______________________________________________
> > > xfs mailing list
> > > xfs@xxxxxxxxxxx
> > > http://oss.sgi.com/mailman/listinfo/xfs
> >
> > _______________________________________________
> > xfs mailing list
> > xfs@xxxxxxxxxxx
> > http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
