* [PATCH] io/mremap: modify argument errors of mremap command
@ 2016-03-16 9:31 Zorro Lang
2016-03-16 12:31 ` Brian Foster
0 siblings, 1 reply; 3+ messages in thread
From: Zorro Lang @ 2016-03-16 9:31 UTC (permalink / raw)
To: xfs; +Cc: Zorro Lang
There're 2 argument errors in mremap command:
1. If run "mremap 1024 8192", it won't return error and try to
do "mremap 1024" silently.
2. The "-f" option can't be used, due to it need a new address
argument as the fifth argument of mremap() syscall(man mremap).
This patch try to fix above two problems.
Signed-off-by: Zorro Lang <zlang@redhat.com>
---
Hi,
I'm not sure pass a address argument to mremap command is right or
wrong. And maybe I should use strtoul() to instead of cvtnum() for
address argument.
At least I did some simple test with this patch on my machine, results are:
"mremap -f 139946004389888 2048" PASS
"mremap -f 4096k 2048" PASS
"mremap -f 1g 2048" PASS
"mremap -f 4096 2048" FAIL as 'Permission denied'
What do you think of this patch?
Thanks,
Zorro
io/mmap.c | 22 +++++++++++++++-------
man/man8/xfs_io.8 | 5 ++++-
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/io/mmap.c b/io/mmap.c
index 5970069..672ee61 100644
--- a/io/mmap.c
+++ b/io/mmap.c
@@ -589,7 +589,7 @@ mremap_help(void)
"\n"
" Resizes the mappping, growing or shrinking from the current size.\n"
" The default stored value is 'X', repeated to fill the range specified.\n"
-" -f -- use the MREMAP_FIXED flag\n"
+" -f <new_address> -- use the MREMAP_FIXED flag to mremap on the new address\n"
" -m -- use the MREMAP_MAYMOVE flag\n"
"\n"));
}
@@ -600,15 +600,18 @@ mremap_f(
char **argv)
{
ssize_t new_length;
- void *new_addr;
+ void *new_addr = NULL;
int flags = 0;
int c;
size_t blocksize, sectsize;
- while ((c = getopt(argc, argv, "fm")) != EOF) {
+ init_cvtnum(&blocksize, §size);
+
+ while ((c = getopt(argc, argv, "f:m")) != EOF) {
switch (c) {
case 'f':
flags = MREMAP_FIXED|MREMAP_MAYMOVE;
+ new_addr = (void *)cvtnum(blocksize, sectsize, optarg);
break;
case 'm':
flags = MREMAP_MAYMOVE;
@@ -618,7 +621,9 @@ mremap_f(
}
}
- init_cvtnum(&blocksize, §size);
+ if (optind != argc - 1)
+ return command_usage(&mremap_cmd);
+
new_length = cvtnum(blocksize, sectsize, argv[optind]);
if (new_length < 0) {
printf(_("non-numeric offset argument -- %s\n"),
@@ -626,7 +631,10 @@ mremap_f(
return 0;
}
- new_addr = mremap(mapping->addr, mapping->length, new_length, flags);
+ if (!new_addr)
+ new_addr = mremap(mapping->addr, mapping->length, new_length, flags);
+ else
+ new_addr = mremap(mapping->addr, mapping->length, new_length, flags, new_addr);
if (new_addr == MAP_FAILED)
perror("mremap");
else {
@@ -697,9 +705,9 @@ mmap_init(void)
mremap_cmd.altname = "mrm";
mremap_cmd.cfunc = mremap_f;
mremap_cmd.argmin = 1;
- mremap_cmd.argmax = 2;
+ mremap_cmd.argmax = 3;
mremap_cmd.flags = CMD_NOFILE_OK | CMD_FOREIGN_OK;
- mremap_cmd.args = _("[-m|-f] newsize");
+ mremap_cmd.args = _("[-m|-f <new_address>] newsize");
mremap_cmd.oneline =
_("alters the size of the current memory mapping");
mremap_cmd.help = mremap_help;
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 33fbe6a..984b551 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -581,7 +581,7 @@ See the
.B mmap
command.
.TP
-.BI "mremap [ \-f ] [ \-m ] " new_length
+.BI "mremap [ \-f <new_address> ] [ \-m ] " new_length
Changes the current mapping size to
.IR new_length .
Whether the mapping may be moved is controlled by the flags passed;
@@ -589,6 +589,9 @@ MREMAP_FIXED
.RB ( \-f ),
or MREMAP_MAYMOVE
.RB ( \-m ).
+.IR new_length
+specifies a page-aligned address to which the mapping must be moved. It
+can be setted to 139946004389888, 4096k or 1g etc.
.TP
.B mrm
See the
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] io/mremap: modify argument errors of mremap command
2016-03-16 9:31 [PATCH] io/mremap: modify argument errors of mremap command Zorro Lang
@ 2016-03-16 12:31 ` Brian Foster
2016-03-16 14:00 ` Zorro Lang
0 siblings, 1 reply; 3+ messages in thread
From: Brian Foster @ 2016-03-16 12:31 UTC (permalink / raw)
To: Zorro Lang; +Cc: xfs
On Wed, Mar 16, 2016 at 05:31:47PM +0800, Zorro Lang wrote:
> There're 2 argument errors in mremap command:
> 1. If run "mremap 1024 8192", it won't return error and try to
> do "mremap 1024" silently.
>
> 2. The "-f" option can't be used, due to it need a new address
> argument as the fifth argument of mremap() syscall(man mremap).
>
> This patch try to fix above two problems.
>
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
A couple nits...
>
> Hi,
>
> I'm not sure pass a address argument to mremap command is right or
> wrong. And maybe I should use strtoul() to instead of cvtnum() for
> address argument.
>
> At least I did some simple test with this patch on my machine, results are:
> "mremap -f 139946004389888 2048" PASS
> "mremap -f 4096k 2048" PASS
> "mremap -f 1g 2048" PASS
> "mremap -f 4096 2048" FAIL as 'Permission denied'
>
> What do you think of this patch?
>
> Thanks,
> Zorro
>
>
> io/mmap.c | 22 +++++++++++++++-------
> man/man8/xfs_io.8 | 5 ++++-
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/io/mmap.c b/io/mmap.c
> index 5970069..672ee61 100644
> --- a/io/mmap.c
> +++ b/io/mmap.c
> @@ -589,7 +589,7 @@ mremap_help(void)
> "\n"
> " Resizes the mappping, growing or shrinking from the current size.\n"
> " The default stored value is 'X', repeated to fill the range specified.\n"
> -" -f -- use the MREMAP_FIXED flag\n"
> +" -f <new_address> -- use the MREMAP_FIXED flag to mremap on the new address\n"
Exceeds 80 chars.
> " -m -- use the MREMAP_MAYMOVE flag\n"
> "\n"));
> }
> @@ -600,15 +600,18 @@ mremap_f(
> char **argv)
> {
> ssize_t new_length;
> - void *new_addr;
> + void *new_addr = NULL;
> int flags = 0;
> int c;
> size_t blocksize, sectsize;
>
> - while ((c = getopt(argc, argv, "fm")) != EOF) {
> + init_cvtnum(&blocksize, §size);
> +
> + while ((c = getopt(argc, argv, "f:m")) != EOF) {
> switch (c) {
> case 'f':
> flags = MREMAP_FIXED|MREMAP_MAYMOVE;
> + new_addr = (void *)cvtnum(blocksize, sectsize, optarg);
> break;
> case 'm':
> flags = MREMAP_MAYMOVE;
> @@ -618,7 +621,9 @@ mremap_f(
> }
> }
>
> - init_cvtnum(&blocksize, §size);
> + if (optind != argc - 1)
> + return command_usage(&mremap_cmd);
> +
> new_length = cvtnum(blocksize, sectsize, argv[optind]);
> if (new_length < 0) {
> printf(_("non-numeric offset argument -- %s\n"),
> @@ -626,7 +631,10 @@ mremap_f(
> return 0;
> }
>
> - new_addr = mremap(mapping->addr, mapping->length, new_length, flags);
> + if (!new_addr)
> + new_addr = mremap(mapping->addr, mapping->length, new_length, flags);
> + else
> + new_addr = mremap(mapping->addr, mapping->length, new_length, flags, new_addr);
Both mremap() calls above exceed 80 chars. Otherwise, this looks Ok to
me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> if (new_addr == MAP_FAILED)
> perror("mremap");
> else {
> @@ -697,9 +705,9 @@ mmap_init(void)
> mremap_cmd.altname = "mrm";
> mremap_cmd.cfunc = mremap_f;
> mremap_cmd.argmin = 1;
> - mremap_cmd.argmax = 2;
> + mremap_cmd.argmax = 3;
> mremap_cmd.flags = CMD_NOFILE_OK | CMD_FOREIGN_OK;
> - mremap_cmd.args = _("[-m|-f] newsize");
> + mremap_cmd.args = _("[-m|-f <new_address>] newsize");
> mremap_cmd.oneline =
> _("alters the size of the current memory mapping");
> mremap_cmd.help = mremap_help;
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 33fbe6a..984b551 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -581,7 +581,7 @@ See the
> .B mmap
> command.
> .TP
> -.BI "mremap [ \-f ] [ \-m ] " new_length
> +.BI "mremap [ \-f <new_address> ] [ \-m ] " new_length
> Changes the current mapping size to
> .IR new_length .
> Whether the mapping may be moved is controlled by the flags passed;
> @@ -589,6 +589,9 @@ MREMAP_FIXED
> .RB ( \-f ),
> or MREMAP_MAYMOVE
> .RB ( \-m ).
> +.IR new_length
> +specifies a page-aligned address to which the mapping must be moved. It
> +can be setted to 139946004389888, 4096k or 1g etc.
> .TP
> .B mrm
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] io/mremap: modify argument errors of mremap command
2016-03-16 12:31 ` Brian Foster
@ 2016-03-16 14:00 ` Zorro Lang
0 siblings, 0 replies; 3+ messages in thread
From: Zorro Lang @ 2016-03-16 14:00 UTC (permalink / raw)
To: Brian Foster; +Cc: xfs
On Wed, Mar 16, 2016 at 08:31:26AM -0400, Brian Foster wrote:
> On Wed, Mar 16, 2016 at 05:31:47PM +0800, Zorro Lang wrote:
> > There're 2 argument errors in mremap command:
> > 1. If run "mremap 1024 8192", it won't return error and try to
> > do "mremap 1024" silently.
> >
> > 2. The "-f" option can't be used, due to it need a new address
> > argument as the fifth argument of mremap() syscall(man mremap).
> >
> > This patch try to fix above two problems.
> >
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
>
> A couple nits...
>
> >
> > Hi,
> >
> > I'm not sure pass a address argument to mremap command is right or
> > wrong. And maybe I should use strtoul() to instead of cvtnum() for
> > address argument.
> >
> > At least I did some simple test with this patch on my machine, results are:
> > "mremap -f 139946004389888 2048" PASS
> > "mremap -f 4096k 2048" PASS
> > "mremap -f 1g 2048" PASS
> > "mremap -f 4096 2048" FAIL as 'Permission denied'
> >
> > What do you think of this patch?
> >
> > Thanks,
> > Zorro
> >
> >
> > io/mmap.c | 22 +++++++++++++++-------
> > man/man8/xfs_io.8 | 5 ++++-
> > 2 files changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/io/mmap.c b/io/mmap.c
> > index 5970069..672ee61 100644
> > --- a/io/mmap.c
> > +++ b/io/mmap.c
> > @@ -589,7 +589,7 @@ mremap_help(void)
> > "\n"
> > " Resizes the mappping, growing or shrinking from the current size.\n"
> > " The default stored value is 'X', repeated to fill the range specified.\n"
> > -" -f -- use the MREMAP_FIXED flag\n"
> > +" -f <new_address> -- use the MREMAP_FIXED flag to mremap on the new address\n"
>
> Exceeds 80 chars.
>
> > " -m -- use the MREMAP_MAYMOVE flag\n"
> > "\n"));
> > }
> > @@ -600,15 +600,18 @@ mremap_f(
> > char **argv)
> > {
> > ssize_t new_length;
> > - void *new_addr;
> > + void *new_addr = NULL;
> > int flags = 0;
> > int c;
> > size_t blocksize, sectsize;
> >
> > - while ((c = getopt(argc, argv, "fm")) != EOF) {
> > + init_cvtnum(&blocksize, §size);
> > +
> > + while ((c = getopt(argc, argv, "f:m")) != EOF) {
> > switch (c) {
> > case 'f':
> > flags = MREMAP_FIXED|MREMAP_MAYMOVE;
> > + new_addr = (void *)cvtnum(blocksize, sectsize, optarg);
> > break;
> > case 'm':
> > flags = MREMAP_MAYMOVE;
> > @@ -618,7 +621,9 @@ mremap_f(
> > }
> > }
> >
> > - init_cvtnum(&blocksize, §size);
> > + if (optind != argc - 1)
> > + return command_usage(&mremap_cmd);
> > +
> > new_length = cvtnum(blocksize, sectsize, argv[optind]);
> > if (new_length < 0) {
> > printf(_("non-numeric offset argument -- %s\n"),
> > @@ -626,7 +631,10 @@ mremap_f(
> > return 0;
> > }
> >
> > - new_addr = mremap(mapping->addr, mapping->length, new_length, flags);
> > + if (!new_addr)
> > + new_addr = mremap(mapping->addr, mapping->length, new_length, flags);
> > + else
> > + new_addr = mremap(mapping->addr, mapping->length, new_length, flags, new_addr);
>
> Both mremap() calls above exceed 80 chars. Otherwise, this looks Ok to
> me:
>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
Hi Brian,
Thanks for your review. I will send a V2 patch to fix these "exceed 80
chars" errors.
Thanks,
Zorro
>
> > if (new_addr == MAP_FAILED)
> > perror("mremap");
> > else {
> > @@ -697,9 +705,9 @@ mmap_init(void)
> > mremap_cmd.altname = "mrm";
> > mremap_cmd.cfunc = mremap_f;
> > mremap_cmd.argmin = 1;
> > - mremap_cmd.argmax = 2;
> > + mremap_cmd.argmax = 3;
> > mremap_cmd.flags = CMD_NOFILE_OK | CMD_FOREIGN_OK;
> > - mremap_cmd.args = _("[-m|-f] newsize");
> > + mremap_cmd.args = _("[-m|-f <new_address>] newsize");
> > mremap_cmd.oneline =
> > _("alters the size of the current memory mapping");
> > mremap_cmd.help = mremap_help;
> > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> > index 33fbe6a..984b551 100644
> > --- a/man/man8/xfs_io.8
> > +++ b/man/man8/xfs_io.8
> > @@ -581,7 +581,7 @@ See the
> > .B mmap
> > command.
> > .TP
> > -.BI "mremap [ \-f ] [ \-m ] " new_length
> > +.BI "mremap [ \-f <new_address> ] [ \-m ] " new_length
> > Changes the current mapping size to
> > .IR new_length .
> > Whether the mapping may be moved is controlled by the flags passed;
> > @@ -589,6 +589,9 @@ MREMAP_FIXED
> > .RB ( \-f ),
> > or MREMAP_MAYMOVE
> > .RB ( \-m ).
> > +.IR new_length
> > +specifies a page-aligned address to which the mapping must be moved. It
> > +can be setted to 139946004389888, 4096k or 1g etc.
> > .TP
> > .B mrm
> > See the
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-03-16 14:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-16 9:31 [PATCH] io/mremap: modify argument errors of mremap command Zorro Lang
2016-03-16 12:31 ` Brian Foster
2016-03-16 14:00 ` Zorro Lang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox