On 11/08/2017 10:27 PM, Eric Sandeen wrote:
> On 10/10/17 5:58 AM, Goldwyn Rodrigues wrote:
>> From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
>>
>> This allows to make pwritev2() calls with RWF_NOWAIT,
>> which would fail in case the call blocks.
>>
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
>>
>> 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
>>
--
Goldwyn
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
