[PATCH v3 1/3] xfs_io: Add support for pwritev2()

[PATCH v3 1/3] xfs_io: Add support for pwritev2()

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>

---
 configure.ac          |  1 +
 include/builddefs.in  |  1 +
 io/Makefile           |  4 ++++
 io/pwrite.c           | 43 ++++++++++++++++++++++++++++++-------------
 m4/package_libcdev.m4 | 16 ++++++++++++++++
 5 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4161c3b4..2320e3e3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,7 @@ AC_HAVE_GETMNTENT
 AC_HAVE_GETMNTINFO
 AC_HAVE_FALLOCATE
 AC_HAVE_FIEMAP
+AC_HAVE_PWRITEV2
 AC_HAVE_PREADV
 AC_HAVE_COPY_FILE_RANGE
 AC_HAVE_SYNC_FILE_RANGE
diff --git a/include/builddefs.in b/include/builddefs.in
index ec630bd9..cd58ea8e 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -103,6 +103,7 @@ HAVE_GETMNTINFO = @have_getmntinfo@
 HAVE_FALLOCATE = @have_fallocate@
 HAVE_FIEMAP = @have_fiemap@
 HAVE_PREADV = @have_preadv@
+HAVE_PWRITEV2 = @have_pwritev2@
 HAVE_COPY_FILE_RANGE = @have_copy_file_range@
 HAVE_SYNC_FILE_RANGE = @have_sync_file_range@
 HAVE_SYNCFS = @have_syncfs@
diff --git a/io/Makefile b/io/Makefile
index 47b0a669..050d6bd0 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -90,6 +90,10 @@ ifeq ($(HAVE_PREADV),yes)
 LCFLAGS += -DHAVE_PREADV -DHAVE_PWRITEV
 endif
 
+ifeq ($(HAVE_PWRITEV2),yes)
+LCFLAGS += -DHAVE_PWRITEV2
+endif
+
 ifeq ($(HAVE_READDIR),yes)
 CFILES += readdir.c
 LCFLAGS += -DHAVE_READDIR
diff --git a/io/pwrite.c b/io/pwrite.c
index 1c5dfca1..5ceb26c7 100644
--- a/io/pwrite.c
+++ b/io/pwrite.c
@@ -62,7 +62,8 @@ do_pwritev(
 	int		fd,
 	off64_t		offset,
 	ssize_t		count,
-	ssize_t		buffer_size)
+	ssize_t		buffer_size,
+	int 		pwritev2_flags)
 {
 	int vecs = 0;
 	ssize_t oldlen = 0;
@@ -81,7 +82,14 @@ do_pwritev(
 	} else {
 		vecs = vectors;
 	}
+#ifdef HAVE_PWRITEV2
+	if (pwritev2_flags)
+		bytes = pwritev2(fd, iov, vectors, offset, pwritev2_flags);
+	else
+		bytes = pwritev(fd, iov, vectors, offset);
+#else
 	bytes = pwritev(fd, iov, vectors, offset);
+#endif
 
 	/* restore trimmed iov */
 	if (oldlen)
@@ -98,12 +106,13 @@ do_pwrite(
 	int		fd,
 	off64_t		offset,
 	ssize_t		count,
-	ssize_t		buffer_size)
+	ssize_t		buffer_size,
+	int		pwritev2_flags)
 {
 	if (!vectors)
 		return pwrite(fd, buffer, min(count, buffer_size), offset);
 
-	return do_pwritev(fd, offset, count, buffer_size);
+	return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags);
 }
 
 static int
@@ -111,7 +120,8 @@ write_random(
 	off64_t		offset,
 	long long	count,
 	unsigned int	seed,
-	long long	*total)
+	long long	*total,
+	int 		pwritev2_flags)
 {
 	off64_t		off, range;
 	ssize_t		bytes;
@@ -133,7 +143,8 @@ write_random(
 				buffersize;
 		else
 			off = offset;
-		bytes = do_pwrite(file->fd, off, buffersize, buffersize);
+		bytes = do_pwrite(file->fd, off, buffersize, buffersize,
+				pwritev2_flags);
 		if (bytes == 0)
 			break;
 		if (bytes < 0) {
@@ -153,7 +164,8 @@ static int
 write_backward(
 	off64_t		offset,
 	long long	*count,
-	long long	*total)
+	long long	*total,
+	int		pwritev2_flags)
 {
 	off64_t		end, off = offset;
 	ssize_t		bytes = 0, bytes_requested;
@@ -171,7 +183,8 @@ write_backward(
 	if ((bytes_requested = (off % buffersize))) {
 		bytes_requested = min(cnt, bytes_requested);
 		off -= bytes_requested;
-		bytes = do_pwrite(file->fd, off, bytes_requested, buffersize);
+		bytes = do_pwrite(file->fd, off, bytes_requested, buffersize,
+				pwritev2_flags);
 		if (bytes == 0)
 			return ops;
 		if (bytes < 0) {
@@ -189,7 +202,8 @@ write_backward(
 	while (cnt > end) {
 		bytes_requested = min(cnt, buffersize);
 		off -= bytes_requested;
-		bytes = do_pwrite(file->fd, off, cnt, buffersize);
+		bytes = do_pwrite(file->fd, off, cnt, buffersize,
+				pwritev2_flags);
 		if (bytes == 0)
 			break;
 		if (bytes < 0) {
@@ -212,7 +226,8 @@ write_buffer(
 	size_t		bs,
 	int		fd,
 	off64_t		skip,
-	long long	*total)
+	long long	*total,
+	int		pwritev2_flags)
 {
 	ssize_t		bytes;
 	long long	bar = min(bs, count);
@@ -224,7 +239,7 @@ write_buffer(
 			if (read_buffer(fd, skip + *total, bs, &bar, 0, 1) < 0)
 				break;
 		}
-		bytes = do_pwrite(file->fd, offset, count, bar);
+		bytes = do_pwrite(file->fd, offset, count, bar, pwritev2_flags);
 		if (bytes == 0)
 			break;
 		if (bytes < 0) {
@@ -258,6 +273,7 @@ pwrite_f(
 	int		Cflag, qflag, uflag, dflag, wflag, Wflag;
 	int		direction = IO_FORWARD;
 	int		c, fd = -1;
+	int		pwritev2_flags = 0;
 
 	Cflag = qflag = uflag = dflag = wflag = Wflag = 0;
 	init_cvtnum(&fsblocksize, &fssectsize);
@@ -365,13 +381,14 @@ pwrite_f(
 	case IO_RANDOM:
 		if (!zeed)	/* srandom seed */
 			zeed = time(NULL);
-		c = write_random(offset, count, zeed, &total);
+		c = write_random(offset, count, zeed, &total, pwritev2_flags);
 		break;
 	case IO_FORWARD:
-		c = write_buffer(offset, count, bsize, fd, skip, &total);
+		c = write_buffer(offset, count, bsize, fd, skip, &total,
+				pwritev2_flags);
 		break;
 	case IO_BACKWARD:
-		c = write_backward(offset, &count, &total);
+		c = write_backward(offset, &count, &total, pwritev2_flags);
 		break;
 	default:
 		total = 0;
diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
index fa5b6397..48da0783 100644
--- a/m4/package_libcdev.m4
+++ b/m4/package_libcdev.m4
@@ -146,6 +146,22 @@ AC_DEFUN([AC_HAVE_PREADV],
     AC_SUBST(have_preadv)
   ])
 
+#
+# Check if we have a pwritev2 libc call (Linux)
+#
+AC_DEFUN([AC_HAVE_PWRITEV2],
+  [ AC_MSG_CHECKING([for pwritev2])
+    AC_TRY_LINK([
+#define _BSD_SOURCE
+#include <sys/uio.h>
+    ], [
+         pwritev2(0, 0, 0, 0, 0);
+    ], have_pwritev2=yes
+       AC_MSG_RESULT(yes),
+       AC_MSG_RESULT(no))
+    AC_SUBST(have_pwritev2)
+  ])
+
 #
 # Check if we have a copy_file_range system call (Linux)
 #
-- 
2.14.2

[PATCH v3 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Goldwyn Rodrigues]

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"));
 }
 
@@ -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

Re: [PATCH v3 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Brian Foster]

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

Re: [PATCH v3 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Eric Sandeen]

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
> 

Re: [PATCH v3 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Goldwyn Rodrigues]

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
>>

-- 
Goldwyn

Re: [PATCH v3 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Eric Sandeen]

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

[PATCH v3 3/3] xfs_io: Allow partial writes

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Partial writes are performed when there is an error midway
while performing the I/O. Perform the write exactly once and
return the number of bytes written so far, until the error.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 io/io.h           |  1 +
 io/pwrite.c       | 25 ++++++++++++++++++++++++-
 man/man8/xfs_io.8 |  3 +++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/io/io.h b/io/io.h
index 6a0fe657..3862985f 100644
--- a/io/io.h
+++ b/io/io.h
@@ -25,6 +25,7 @@
 #define IO_RANDOM	( 0)
 #define IO_FORWARD	( 1)
 #define IO_BACKWARD	(-1)
+#define IO_ONCE		( 2)
 
 /*
  * File descriptor options
diff --git a/io/pwrite.c b/io/pwrite.c
index e06dfb46..7f4dbef9 100644
--- a/io/pwrite.c
+++ b/io/pwrite.c
@@ -47,6 +47,7 @@ pwrite_help(void)
 " -W   -- call fsync(2) at the end (included in timing results)\n"
 " -B   -- write backwards through the range from offset (backwards N bytes)\n"
 " -F   -- write forwards through the range of bytes from offset (default)\n"
+" -O   -- perform pwrite call once and return (maybe partial) bytes written\n"
 " -R   -- write at random offsets in the specified range of bytes\n"
 " -Z N -- zeed the random number generator (used when writing randomly)\n"
 "         (heh, zorry, the -s/-S arguments were already in use in pwrite)\n"
@@ -261,6 +262,22 @@ write_buffer(
 	return ops;
 }
 
+static int
+write_once(
+	off64_t		offset,
+	long long	count,
+	long long	*total,
+	int		pwritev2_flags)
+{
+	size_t bytes;
+	bytes = do_pwrite(file->fd, offset, count, count, pwritev2_flags);
+	if (bytes < 0)
+		return -1;
+	*total = bytes;
+	return 1;
+}
+
+
 static int
 pwrite_f(
 	int		argc,
@@ -282,7 +299,7 @@ pwrite_f(
 	init_cvtnum(&fsblocksize, &fssectsize);
 	bsize = fsblocksize;
 
-	while ((c = getopt(argc, argv, "b:BCdf:Fi:NqRs:S:uV:wWZ:")) != EOF) {
+	while ((c = getopt(argc, argv, "b:BCdf:Fi:NqRs:OS:uV:wWZ:")) != EOF) {
 		switch (c) {
 		case 'b':
 			tmp = cvtnum(fsblocksize, fssectsize, optarg);
@@ -304,6 +321,9 @@ pwrite_f(
 		case 'R':
 			direction = IO_RANDOM;
 			break;
+		case 'O':
+			direction = IO_ONCE;
+			break;
 		case 'd':
 			dflag = 1;
 			break;
@@ -398,6 +418,9 @@ pwrite_f(
 	case IO_BACKWARD:
 		c = write_backward(offset, &count, &total, pwritev2_flags);
 		break;
+	case IO_ONCE:
+		c = write_once(offset, count, &total, pwritev2_flags);
+		break;
 	default:
 		total = 0;
 		ASSERT(0);
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 9c58914f..b7c0f099 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -263,6 +263,9 @@ write the buffers in a reserve sequential direction.
 .B \-R
 write the buffers in the give range in a random order.
 .TP
+.B \-O
+perform pwrite once and return the (maybe partial) bytes written.
+.TP
 .B \-Z seed
 specify the random number seed used for random write
 .TP
-- 
2.14.2

Re: [PATCH v3 1/3] xfs_io: Add support for pwritev2()

[Brian Foster]

On Tue, Oct 10, 2017 at 05:58:00AM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> ---
>  configure.ac          |  1 +
>  include/builddefs.in  |  1 +
>  io/Makefile           |  4 ++++
>  io/pwrite.c           | 43 ++++++++++++++++++++++++++++++-------------
>  m4/package_libcdev.m4 | 16 ++++++++++++++++
>  5 files changed, 52 insertions(+), 13 deletions(-)
> 

Looks Ok to me:

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

> diff --git a/configure.ac b/configure.ac
> index 4161c3b4..2320e3e3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -132,6 +132,7 @@ AC_HAVE_GETMNTENT
>  AC_HAVE_GETMNTINFO
>  AC_HAVE_FALLOCATE
>  AC_HAVE_FIEMAP
> +AC_HAVE_PWRITEV2
>  AC_HAVE_PREADV
>  AC_HAVE_COPY_FILE_RANGE
>  AC_HAVE_SYNC_FILE_RANGE
> diff --git a/include/builddefs.in b/include/builddefs.in
> index ec630bd9..cd58ea8e 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -103,6 +103,7 @@ HAVE_GETMNTINFO = @have_getmntinfo@
>  HAVE_FALLOCATE = @have_fallocate@
>  HAVE_FIEMAP = @have_fiemap@
>  HAVE_PREADV = @have_preadv@
> +HAVE_PWRITEV2 = @have_pwritev2@
>  HAVE_COPY_FILE_RANGE = @have_copy_file_range@
>  HAVE_SYNC_FILE_RANGE = @have_sync_file_range@
>  HAVE_SYNCFS = @have_syncfs@
> diff --git a/io/Makefile b/io/Makefile
> index 47b0a669..050d6bd0 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -90,6 +90,10 @@ ifeq ($(HAVE_PREADV),yes)
>  LCFLAGS += -DHAVE_PREADV -DHAVE_PWRITEV
>  endif
>  
> +ifeq ($(HAVE_PWRITEV2),yes)
> +LCFLAGS += -DHAVE_PWRITEV2
> +endif
> +
>  ifeq ($(HAVE_READDIR),yes)
>  CFILES += readdir.c
>  LCFLAGS += -DHAVE_READDIR
> diff --git a/io/pwrite.c b/io/pwrite.c
> index 1c5dfca1..5ceb26c7 100644
> --- a/io/pwrite.c
> +++ b/io/pwrite.c
> @@ -62,7 +62,8 @@ do_pwritev(
>  	int		fd,
>  	off64_t		offset,
>  	ssize_t		count,
> -	ssize_t		buffer_size)
> +	ssize_t		buffer_size,
> +	int 		pwritev2_flags)
>  {
>  	int vecs = 0;
>  	ssize_t oldlen = 0;
> @@ -81,7 +82,14 @@ do_pwritev(
>  	} else {
>  		vecs = vectors;
>  	}
> +#ifdef HAVE_PWRITEV2
> +	if (pwritev2_flags)
> +		bytes = pwritev2(fd, iov, vectors, offset, pwritev2_flags);
> +	else
> +		bytes = pwritev(fd, iov, vectors, offset);
> +#else
>  	bytes = pwritev(fd, iov, vectors, offset);
> +#endif
>  
>  	/* restore trimmed iov */
>  	if (oldlen)
> @@ -98,12 +106,13 @@ do_pwrite(
>  	int		fd,
>  	off64_t		offset,
>  	ssize_t		count,
> -	ssize_t		buffer_size)
> +	ssize_t		buffer_size,
> +	int		pwritev2_flags)
>  {
>  	if (!vectors)
>  		return pwrite(fd, buffer, min(count, buffer_size), offset);
>  
> -	return do_pwritev(fd, offset, count, buffer_size);
> +	return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags);
>  }
>  
>  static int
> @@ -111,7 +120,8 @@ write_random(
>  	off64_t		offset,
>  	long long	count,
>  	unsigned int	seed,
> -	long long	*total)
> +	long long	*total,
> +	int 		pwritev2_flags)
>  {
>  	off64_t		off, range;
>  	ssize_t		bytes;
> @@ -133,7 +143,8 @@ write_random(
>  				buffersize;
>  		else
>  			off = offset;
> -		bytes = do_pwrite(file->fd, off, buffersize, buffersize);
> +		bytes = do_pwrite(file->fd, off, buffersize, buffersize,
> +				pwritev2_flags);
>  		if (bytes == 0)
>  			break;
>  		if (bytes < 0) {
> @@ -153,7 +164,8 @@ static int
>  write_backward(
>  	off64_t		offset,
>  	long long	*count,
> -	long long	*total)
> +	long long	*total,
> +	int		pwritev2_flags)
>  {
>  	off64_t		end, off = offset;
>  	ssize_t		bytes = 0, bytes_requested;
> @@ -171,7 +183,8 @@ write_backward(
>  	if ((bytes_requested = (off % buffersize))) {
>  		bytes_requested = min(cnt, bytes_requested);
>  		off -= bytes_requested;
> -		bytes = do_pwrite(file->fd, off, bytes_requested, buffersize);
> +		bytes = do_pwrite(file->fd, off, bytes_requested, buffersize,
> +				pwritev2_flags);
>  		if (bytes == 0)
>  			return ops;
>  		if (bytes < 0) {
> @@ -189,7 +202,8 @@ write_backward(
>  	while (cnt > end) {
>  		bytes_requested = min(cnt, buffersize);
>  		off -= bytes_requested;
> -		bytes = do_pwrite(file->fd, off, cnt, buffersize);
> +		bytes = do_pwrite(file->fd, off, cnt, buffersize,
> +				pwritev2_flags);
>  		if (bytes == 0)
>  			break;
>  		if (bytes < 0) {
> @@ -212,7 +226,8 @@ write_buffer(
>  	size_t		bs,
>  	int		fd,
>  	off64_t		skip,
> -	long long	*total)
> +	long long	*total,
> +	int		pwritev2_flags)
>  {
>  	ssize_t		bytes;
>  	long long	bar = min(bs, count);
> @@ -224,7 +239,7 @@ write_buffer(
>  			if (read_buffer(fd, skip + *total, bs, &bar, 0, 1) < 0)
>  				break;
>  		}
> -		bytes = do_pwrite(file->fd, offset, count, bar);
> +		bytes = do_pwrite(file->fd, offset, count, bar, pwritev2_flags);
>  		if (bytes == 0)
>  			break;
>  		if (bytes < 0) {
> @@ -258,6 +273,7 @@ pwrite_f(
>  	int		Cflag, qflag, uflag, dflag, wflag, Wflag;
>  	int		direction = IO_FORWARD;
>  	int		c, fd = -1;
> +	int		pwritev2_flags = 0;
>  
>  	Cflag = qflag = uflag = dflag = wflag = Wflag = 0;
>  	init_cvtnum(&fsblocksize, &fssectsize);
> @@ -365,13 +381,14 @@ pwrite_f(
>  	case IO_RANDOM:
>  		if (!zeed)	/* srandom seed */
>  			zeed = time(NULL);
> -		c = write_random(offset, count, zeed, &total);
> +		c = write_random(offset, count, zeed, &total, pwritev2_flags);
>  		break;
>  	case IO_FORWARD:
> -		c = write_buffer(offset, count, bsize, fd, skip, &total);
> +		c = write_buffer(offset, count, bsize, fd, skip, &total,
> +				pwritev2_flags);
>  		break;
>  	case IO_BACKWARD:
> -		c = write_backward(offset, &count, &total);
> +		c = write_backward(offset, &count, &total, pwritev2_flags);
>  		break;
>  	default:
>  		total = 0;
> diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
> index fa5b6397..48da0783 100644
> --- a/m4/package_libcdev.m4
> +++ b/m4/package_libcdev.m4
> @@ -146,6 +146,22 @@ AC_DEFUN([AC_HAVE_PREADV],
>      AC_SUBST(have_preadv)
>    ])
>  
> +#
> +# Check if we have a pwritev2 libc call (Linux)
> +#
> +AC_DEFUN([AC_HAVE_PWRITEV2],
> +  [ AC_MSG_CHECKING([for pwritev2])
> +    AC_TRY_LINK([
> +#define _BSD_SOURCE
> +#include <sys/uio.h>
> +    ], [
> +         pwritev2(0, 0, 0, 0, 0);
> +    ], have_pwritev2=yes
> +       AC_MSG_RESULT(yes),
> +       AC_MSG_RESULT(no))
> +    AC_SUBST(have_pwritev2)
> +  ])
> +
>  #
>  # Check if we have a copy_file_range system call (Linux)
>  #
> -- 
> 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