Message ID | 20171010105802.31353-2-rgoldwyn@suse.de (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Tue, Oct 10, 2017 at 05:58:01AM -0500, Goldwyn Rodrigues wrote: > From: Goldwyn Rodrigues <rgoldwyn@suse.com> > > This allows to make pwritev2() calls with RWF_NOWAIT, > which would fail in case the call blocks. > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Changes since v2: > - ifdef around -N which set RWF_NOWAIT > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > io/pwrite.c | 10 +++++++++- > man/man8/xfs_io.8 | 6 ++++++ > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/io/pwrite.c b/io/pwrite.c > index 5ceb26c7..e06dfb46 100644 > --- a/io/pwrite.c > +++ b/io/pwrite.c > @@ -53,6 +53,9 @@ pwrite_help(void) > #ifdef HAVE_PWRITEV > " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n" > #endif > +#ifdef HAVE_PWRITEV2 > +" -N -- Perform the pwritev2() with RWF_NOWAIT\n" > +#endif > "\n")); > } > > @@ -279,7 +282,7 @@ pwrite_f( > init_cvtnum(&fsblocksize, &fssectsize); > bsize = fsblocksize; > > - while ((c = getopt(argc, argv, "b:BCdf:Fi:qRs:S:uV:wWZ:")) != EOF) { > + while ((c = getopt(argc, argv, "b:BCdf:Fi:NqRs:S:uV:wWZ:")) != EOF) { > switch (c) { > case 'b': > tmp = cvtnum(fsblocksize, fssectsize, optarg); > @@ -308,6 +311,11 @@ pwrite_f( > case 'i': > infile = optarg; > break; > +#ifdef HAVE_PWRITEV2 > + case 'N': > + pwritev2_flags |= RWF_NOWAIT; > + break; > +#endif > case 's': > skip = cvtnum(fsblocksize, fssectsize, optarg); > if (skip < 0) { > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 > index 0fd9b951..9c58914f 100644 > --- a/man/man8/xfs_io.8 > +++ b/man/man8/xfs_io.8 > @@ -282,6 +282,12 @@ Use the vectored IO write syscall > with a number of blocksize length iovecs. The number of iovecs is set by the > .I vectors > parameter. > +.TP > +.B \-N > +Perform the > +.BR pwritev2 (2) > +call with > +.I RWF_NOWAIT. > .RE > .PD > .TP > -- > 2.14.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" 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 linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/10/17 5:58 AM, Goldwyn Rodrigues wrote: > From: Goldwyn Rodrigues <rgoldwyn@suse.com> > > This allows to make pwritev2() calls with RWF_NOWAIT, > which would fail in case the call blocks. > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Changes since v2: > - ifdef around -N which set RWF_NOWAIT > --- > io/pwrite.c | 10 +++++++++- > man/man8/xfs_io.8 | 6 ++++++ > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/io/pwrite.c b/io/pwrite.c > index 5ceb26c7..e06dfb46 100644 > --- a/io/pwrite.c > +++ b/io/pwrite.c > @@ -53,6 +53,9 @@ pwrite_help(void) > #ifdef HAVE_PWRITEV > " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n" > #endif > +#ifdef HAVE_PWRITEV2 > +" -N -- Perform the pwritev2() with RWF_NOWAIT\n" > +#endif > "\n")); > } This "-N" option didn't get added to the short help: void pwrite_init(void) { pwrite_cmd.name = "pwrite"; pwrite_cmd.altname = "w"; pwrite_cmd.cfunc = pwrite_f; pwrite_cmd.argmin = 2; pwrite_cmd.argmax = -1; pwrite_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK; pwrite_cmd.args = _("[-i infile [-d] [-s skip]] [-b bs] [-S seed] [-wW] [-FBR [-Z N]] [-V N] off len"); Is there any clean way to do that conditionally on the #ifdef as is done for long help? Otherwise just more #ifdefs I guess. > @@ -279,7 +282,7 @@ pwrite_f( > init_cvtnum(&fsblocksize, &fssectsize); > bsize = fsblocksize; > > - while ((c = getopt(argc, argv, "b:BCdf:Fi:qRs:S:uV:wWZ:")) != EOF) { > + while ((c = getopt(argc, argv, "b:BCdf:Fi:NqRs:S:uV:wWZ:")) != EOF) { > switch (c) { > case 'b': > tmp = cvtnum(fsblocksize, fssectsize, optarg); > @@ -308,6 +311,11 @@ pwrite_f( > case 'i': > infile = optarg; > break; > +#ifdef HAVE_PWRITEV2 > + case 'N': > + pwritev2_flags |= RWF_NOWAIT; > + break; > +#endif If pwritev2 isn't present at build time, specifying -N gives somewhat unexpected behavior: xfs_io> pwrite -N 0 1k pwrite [-i infile [-d] [-s skip]] [-b bs] [-S seed] [-wW] [-FBR [-Z N]] [-V N] off len -- writes a number of bytes at a specified offset xfs_io> vs a wholly unknown option: xfs_io> pwrite -K 0 1k pwrite: invalid option -- 'K' xfs_io> because you have 'N' in the getopt string. I wonder if there's a better way to handle it besides moar ifdefs ... I guess this wouldn't be terrible: + case 'N': +#ifdef HAVE_PWRITEV2 + pwritev2_flags |= RWF_NOWAIT; +#else + printf(_("Not built with pwritev2 functionality\n")); +#endif + break; > case 's': > skip = cvtnum(fsblocksize, fssectsize, optarg); > if (skip < 0) { > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 > index 0fd9b951..9c58914f 100644 > --- a/man/man8/xfs_io.8 > +++ b/man/man8/xfs_io.8 > @@ -282,6 +282,12 @@ Use the vectored IO write syscall > with a number of blocksize length iovecs. The number of iovecs is set by the > .I vectors > parameter. > +.TP > +.B \-N > +Perform the > +.BR pwritev2 (2) > +call with > +.I RWF_NOWAIT. I guess maybe this should say something about "if it's built w/ pwritev2 functionality?" I'm less worried about this, tbh, especially if -N gives the explanation above if xfs_io doesn't have the support. -Eric > .RE > .PD > .TP > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/08/2017 10:27 PM, Eric Sandeen wrote: > On 10/10/17 5:58 AM, Goldwyn Rodrigues wrote: >> From: Goldwyn Rodrigues <rgoldwyn@suse.com> >> >> This allows to make pwritev2() calls with RWF_NOWAIT, >> which would fail in case the call blocks. >> >> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> >> >> Changes since v2: >> - ifdef around -N which set RWF_NOWAIT >> --- >> io/pwrite.c | 10 +++++++++- >> man/man8/xfs_io.8 | 6 ++++++ >> 2 files changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/io/pwrite.c b/io/pwrite.c >> index 5ceb26c7..e06dfb46 100644 >> --- a/io/pwrite.c >> +++ b/io/pwrite.c >> @@ -53,6 +53,9 @@ pwrite_help(void) >> #ifdef HAVE_PWRITEV >> " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n" >> #endif >> +#ifdef HAVE_PWRITEV2 >> +" -N -- Perform the pwritev2() with RWF_NOWAIT\n" >> +#endif >> "\n")); >> } > > This "-N" option didn't get added to the short help: Ok, I will do that. > > void > pwrite_init(void) > { > pwrite_cmd.name = "pwrite"; > pwrite_cmd.altname = "w"; > pwrite_cmd.cfunc = pwrite_f; > pwrite_cmd.argmin = 2; > pwrite_cmd.argmax = -1; > pwrite_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK; > pwrite_cmd.args = > _("[-i infile [-d] [-s skip]] [-b bs] [-S seed] [-wW] [-FBR [-Z N]] [-V N] off len"); > > Is there any clean way to do that conditionally on the #ifdef as is done for long > help? Otherwise just more #ifdefs I guess. > >> @@ -279,7 +282,7 @@ pwrite_f( >> init_cvtnum(&fsblocksize, &fssectsize); >> bsize = fsblocksize; >> >> - while ((c = getopt(argc, argv, "b:BCdf:Fi:qRs:S:uV:wWZ:")) != EOF) { >> + while ((c = getopt(argc, argv, "b:BCdf:Fi:NqRs:S:uV:wWZ:")) != EOF) { >> switch (c) { >> case 'b': >> tmp = cvtnum(fsblocksize, fssectsize, optarg); >> @@ -308,6 +311,11 @@ pwrite_f( >> case 'i': >> infile = optarg; >> break; >> +#ifdef HAVE_PWRITEV2 >> + case 'N': >> + pwritev2_flags |= RWF_NOWAIT; >> + break; >> +#endif > > If pwritev2 isn't present at build time, specifying -N gives somewhat unexpected behavior: > > xfs_io> pwrite -N 0 1k > pwrite [-i infile [-d] [-s skip]] [-b bs] [-S seed] [-wW] [-FBR [-Z N]] [-V N] off len -- writes a number of bytes at a specified offset > xfs_io> > > vs a wholly unknown option: > > xfs_io> pwrite -K 0 1k > pwrite: invalid option -- 'K' > xfs_io> > > because you have 'N' in the getopt string. I wonder if there's a better > way to handle it besides moar ifdefs ... I guess this wouldn't be > terrible: > > + case 'N': > +#ifdef HAVE_PWRITEV2 > + pwritev2_flags |= RWF_NOWAIT; > +#else > + printf(_("Not built with pwritev2 functionality\n")); > +#endif > + break; > I had proposed something similar (with another message) in v2, but Dave did not like it. I am fine to make it work either ways. Let me know. Note, We would have to put similar checks for -V option which would add some more ifdefs, which will make it a mess. >> case 's': >> skip = cvtnum(fsblocksize, fssectsize, optarg); >> if (skip < 0) { >> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 >> index 0fd9b951..9c58914f 100644 >> --- a/man/man8/xfs_io.8 >> +++ b/man/man8/xfs_io.8 >> @@ -282,6 +282,12 @@ Use the vectored IO write syscall >> with a number of blocksize length iovecs. The number of iovecs is set by the >> .I vectors >> parameter. >> +.TP >> +.B \-N >> +Perform the >> +.BR pwritev2 (2) >> +call with >> +.I RWF_NOWAIT. > > I guess maybe this should say something about "if it's built w/ pwritev2 functionality?" > I'm less worried about this, tbh, especially if -N gives the explanation above > if xfs_io doesn't have the support. > Yes, I will add that. > -Eric > >> .RE >> .PD >> .TP >>
On 11/9/17 6:25 AM, Goldwyn Rodrigues wrote: > > > On 11/08/2017 10:27 PM, Eric Sandeen wrote: >> On 10/10/17 5:58 AM, Goldwyn Rodrigues wrote: ... >> + case 'N': >> +#ifdef HAVE_PWRITEV2 >> + pwritev2_flags |= RWF_NOWAIT; >> +#else >> + printf(_("Not built with pwritev2 functionality\n")); >> +#endif >> + break; >> > > I had proposed something similar (with another message) in v2, but Dave > did not like it. I am fine to make it work either ways. Let me know. > Note, We would have to put similar checks for -V option which would add > some more ifdefs, which will make it a mess. Oh, right sorry. Dave had suggested that the default case should say: "command -%c not supported" to support all the configurable options, and I agree that's a better solution. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" 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/io/pwrite.c b/io/pwrite.c index 5ceb26c7..e06dfb46 100644 --- a/io/pwrite.c +++ b/io/pwrite.c @@ -53,6 +53,9 @@ pwrite_help(void) #ifdef HAVE_PWRITEV " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n" #endif +#ifdef HAVE_PWRITEV2 +" -N -- Perform the pwritev2() with RWF_NOWAIT\n" +#endif "\n")); } @@ -279,7 +282,7 @@ pwrite_f( init_cvtnum(&fsblocksize, &fssectsize); bsize = fsblocksize; - while ((c = getopt(argc, argv, "b:BCdf:Fi:qRs:S:uV:wWZ:")) != EOF) { + while ((c = getopt(argc, argv, "b:BCdf:Fi:NqRs:S:uV:wWZ:")) != EOF) { switch (c) { case 'b': tmp = cvtnum(fsblocksize, fssectsize, optarg); @@ -308,6 +311,11 @@ pwrite_f( case 'i': infile = optarg; break; +#ifdef HAVE_PWRITEV2 + case 'N': + pwritev2_flags |= RWF_NOWAIT; + break; +#endif case 's': skip = cvtnum(fsblocksize, fssectsize, optarg); if (skip < 0) { diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 index 0fd9b951..9c58914f 100644 --- a/man/man8/xfs_io.8 +++ b/man/man8/xfs_io.8 @@ -282,6 +282,12 @@ Use the vectored IO write syscall with a number of blocksize length iovecs. The number of iovecs is set by the .I vectors parameter. +.TP +.B \-N +Perform the +.BR pwritev2 (2) +call with +.I RWF_NOWAIT. .RE .PD .TP