All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Zorro Lang <zlang@redhat.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] io/mmap: new -s option for mmap command to reserve some free space
Date: Wed, 16 Mar 2016 08:31:14 -0400	[thread overview]
Message-ID: <20160316123113.GB8096@bfoster.bfoster> (raw)
In-Reply-To: <1458119884-17942-1-git-send-email-zlang@redhat.com>

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@redhat.com>
> ---

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:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  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, &sectsize);
> +
> +	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, &sectsize);
>  	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
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2016-03-16 12:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-16  9:18 [PATCH] io/mmap: new -s option for mmap command to reserve some free space Zorro Lang
2016-03-16 12:31 ` Brian Foster [this message]
2016-03-16 13:53   ` Zorro Lang
2016-03-16 14:22     ` Brian Foster
2016-03-16 15:12       ` Zorro Lang
2016-03-16 15:33         ` Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160316123113.GB8096@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=xfs@oss.sgi.com \
    --cc=zlang@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.