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

[PATCH v2 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           | 39 ++++++++++++++++++++++++++-------------
 m4/package_libcdev.m4 | 16 ++++++++++++++++
 5 files changed, 48 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..e7d411bb 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,7 @@ 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 +163,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 +182,7 @@ 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 +200,7 @@ 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 +223,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 +236,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 +270,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 +378,13 @@ 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.1

[PATCH v2 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>
---
 io/pwrite.c       | 8 +++++++-
 man/man8/xfs_io.8 | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/io/pwrite.c b/io/pwrite.c
index e7d411bb..2b85a528 100644
--- a/io/pwrite.c
+++ b/io/pwrite.c
@@ -52,6 +52,9 @@ pwrite_help(void)
 "         (heh, zorry, the -s/-S arguments were already in use in pwrite)\n"
 #ifdef HAVE_PWRITEV
 " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n"
+#ifdef HAVE_PWRITEV2
+" -N   -- Perform the pwritev2() with RWF_NOWAIT\n"
+#endif
 #endif
 "\n"));
 }
@@ -276,7 +279,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);
@@ -305,6 +308,9 @@ pwrite_f(
 		case 'i':
 			infile = optarg;
 			break;
+		case 'N':
+			pwritev2_flags |= RWF_NOWAIT;
+			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.
 .RE
 .PD
 .TP
-- 
2.14.1

[PATCH v2 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>
---
 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 2b85a528..dcf130d6 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"
@@ -258,6 +259,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,
@@ -279,7 +296,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);
@@ -301,6 +318,9 @@ pwrite_f(
 		case 'R':
 			direction = IO_RANDOM;
 			break;
+		case 'O':
+			direction = IO_ONCE;
+			break;
 		case 'd':
 			dflag = 1;
 			break;
@@ -392,6 +412,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.1

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

[Goldwyn Rodrigues]

Hi,

Any response/review comments to these patches?
I have two fstests waiting for these patches to be incorporated.

-- 
Goldwyn

On 09/29/2017 08:00 AM, 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           | 39 ++++++++++++++++++++++++++-------------
>  m4/package_libcdev.m4 | 16 ++++++++++++++++
>  5 files changed, 48 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..e7d411bb 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,7 @@ 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 +163,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 +182,7 @@ 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 +200,7 @@ 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 +223,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 +236,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 +270,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 +378,13 @@ 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)
>  #
> 

-- 
Goldwyn

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

[Brian Foster]

On Fri, Sep 29, 2017 at 08:00:33AM -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           | 39 ++++++++++++++++++++++++++-------------
>  m4/package_libcdev.m4 | 16 ++++++++++++++++
>  5 files changed, 48 insertions(+), 13 deletions(-)
> 
...
> diff --git a/io/pwrite.c b/io/pwrite.c
> index 1c5dfca1..e7d411bb 100644
> --- a/io/pwrite.c
> +++ b/io/pwrite.c
...
> @@ -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,7 @@ write_random(
>  				buffersize;
>  		else
>  			off = offset;
> -		bytes = do_pwrite(file->fd, off, buffersize, buffersize);
> +		bytes = do_pwrite(file->fd, off, buffersize, buffersize, pwritev2_flags);

Looks like a couple or so long lines here and below. We usually wrap
lines at 80 chars. Otherwise this seems fine to me.

Brian

>  		if (bytes == 0)
>  			break;
>  		if (bytes < 0) {
> @@ -153,7 +163,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 +182,7 @@ 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 +200,7 @@ 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 +223,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 +236,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 +270,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 +378,13 @@ 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.1
> 
> --
> 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 v2 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Brian Foster]

On Fri, Sep 29, 2017 at 08:00:34AM -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>
> ---
>  io/pwrite.c       | 8 +++++++-
>  man/man8/xfs_io.8 | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/io/pwrite.c b/io/pwrite.c
> index e7d411bb..2b85a528 100644
> --- a/io/pwrite.c
> +++ b/io/pwrite.c
> @@ -52,6 +52,9 @@ pwrite_help(void)
>  "         (heh, zorry, the -s/-S arguments were already in use in pwrite)\n"
>  #ifdef HAVE_PWRITEV
>  " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n"
> +#ifdef HAVE_PWRITEV2
> +" -N   -- Perform the pwritev2() with RWF_NOWAIT\n"
> +#endif
>  #endif
>  "\n"));
>  }
> @@ -276,7 +279,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);
> @@ -305,6 +308,9 @@ pwrite_f(
>  		case 'i':
>  			infile = optarg;
>  			break;
> +		case 'N':
> +			pwritev2_flags |= RWF_NOWAIT;

This causes a build failure on my local machine due to lack of
RWF_NOWAIT. #ifdef HAVE_PWRITEV2 around this case? Otherwise the code
looks Ok.

Brian

> +			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.
>  .RE
>  .PD
>  .TP
> -- 
> 2.14.1
> 
> --
> 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 v2 3/3] xfs_io: Allow partial writes

[Brian Foster]

On Fri, Sep 29, 2017 at 08:00:35AM -0500, Goldwyn Rodrigues wrote:
> 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: 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 2b85a528..dcf130d6 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"
> @@ -258,6 +259,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,
> @@ -279,7 +296,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);
> @@ -301,6 +318,9 @@ pwrite_f(
>  		case 'R':
>  			direction = IO_RANDOM;
>  			break;
> +		case 'O':
> +			direction = IO_ONCE;
> +			break;
>  		case 'd':
>  			dflag = 1;
>  			break;
> @@ -392,6 +412,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.1
> 
> --
> 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 v2 3/3] xfs_io: Allow partial writes

[Darrick J. Wong]

On Fri, Sep 29, 2017 at 08:00:35AM -0500, Goldwyn Rodrigues wrote:
> 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>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.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 2b85a528..dcf130d6 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"
> @@ -258,6 +259,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,
> @@ -279,7 +296,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);
> @@ -301,6 +318,9 @@ pwrite_f(
>  		case 'R':
>  			direction = IO_RANDOM;
>  			break;
> +		case 'O':
> +			direction = IO_ONCE;
> +			break;
>  		case 'd':
>  			dflag = 1;
>  			break;
> @@ -392,6 +412,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.1
> 
> --
> 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 v2 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Darrick J. Wong]

On Fri, Sep 29, 2017 at 08:00:34AM -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>
> ---
>  io/pwrite.c       | 8 +++++++-
>  man/man8/xfs_io.8 | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/io/pwrite.c b/io/pwrite.c
> index e7d411bb..2b85a528 100644
> --- a/io/pwrite.c
> +++ b/io/pwrite.c
> @@ -52,6 +52,9 @@ pwrite_help(void)
>  "         (heh, zorry, the -s/-S arguments were already in use in pwrite)\n"
>  #ifdef HAVE_PWRITEV
>  " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n"
> +#ifdef HAVE_PWRITEV2
> +" -N   -- Perform the pwritev2() with RWF_NOWAIT\n"
> +#endif

Separate these two? i.e.

#ifdef HAVE_PWRITEV
" -V N..."
#endif
#ifdef HAVE_PWRITEV2
" -N ..."
#endif

They're tested separately in configure, so the ifdefs needn't be nested.

>  #endif
>  "\n"));
>  }
> @@ -276,7 +279,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);
> @@ -305,6 +308,9 @@ pwrite_f(
>  		case 'i':
>  			infile = optarg;
>  			break;
> +		case 'N':
> +			pwritev2_flags |= RWF_NOWAIT;
> +			break;

Needs #ifdef HAVE_PWRITEV2.

If you feel the need to be extra cautious, you could also put:

#ifndef HAVE_PWRITEV2
	assert(pwritev2_flags == 0);
#endif

...just after the getopt parsing to make sure that we never silently
drop RWF_* flags due to code bugs.

--D

>  		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.1
> 
> --
> 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 v2 2/3] xfs_io: Add RWF_NOWAIT to pwritev2()

[Goldwyn Rodrigues]

On 10/09/2017 12:19 PM, Darrick J. Wong wrote:
> On Fri, Sep 29, 2017 at 08:00:34AM -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>
>> ---
>>  io/pwrite.c       | 8 +++++++-
>>  man/man8/xfs_io.8 | 6 ++++++
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/io/pwrite.c b/io/pwrite.c
>> index e7d411bb..2b85a528 100644
>> --- a/io/pwrite.c
>> +++ b/io/pwrite.c
>> @@ -52,6 +52,9 @@ pwrite_help(void)
>>  "         (heh, zorry, the -s/-S arguments were already in use in pwrite)\n"
>>  #ifdef HAVE_PWRITEV
>>  " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n"
>> +#ifdef HAVE_PWRITEV2
>> +" -N   -- Perform the pwritev2() with RWF_NOWAIT\n"
>> +#endif
> 
> Separate these two? i.e.
> 
> #ifdef HAVE_PWRITEV
> " -V N..."
> #endif
> #ifdef HAVE_PWRITEV2
> " -N ..."
> #endif
> 
> They're tested separately in configure, so the ifdefs needn't be nested.
> 
>>  #endif
>>  "\n"));
>>  }
>> @@ -276,7 +279,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);
>> @@ -305,6 +308,9 @@ pwrite_f(
>>  		case 'i':
>>  			infile = optarg;
>>  			break;
>> +		case 'N':
>> +			pwritev2_flags |= RWF_NOWAIT;
>> +			break;
> 
> Needs #ifdef HAVE_PWRITEV2.
> 
> If you feel the need to be extra cautious, you could also put:
> 
> #ifndef HAVE_PWRITEV2
> 	assert(pwritev2_flags == 0);
> #endif
> 
> ...just after the getopt parsing to make sure that we never silently
> drop RWF_* flags due to code bugs.
> 

Instead I am planning to put an #else to #ifdef in the case 'N' to make
sure that people who do not have pwritev2() get the proper message
instead of the command failing without reason.

+               case 'N':
+#ifdef HAVE_PWRITEV2
+                       pwritev2_flags |= RWF_NOWAIT;
+                       break;
+#else
+                       printf(_("-N: Kernel does not support
pwritev2()\n"));
+                       return 0;
+#endif



> --D
> 
>>  		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.1
>>
>> --
>> 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

-- 
Goldwyn

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

[Dave Chinner]

On Mon, Oct 09, 2017 at 04:11:17PM -0500, Goldwyn Rodrigues wrote:
> 
> 
> On 10/09/2017 12:19 PM, Darrick J. Wong wrote:
> > On Fri, Sep 29, 2017 at 08:00:34AM -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>
> >> ---
> >>  io/pwrite.c       | 8 +++++++-
> >>  man/man8/xfs_io.8 | 6 ++++++
> >>  2 files changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/io/pwrite.c b/io/pwrite.c
> >> index e7d411bb..2b85a528 100644
> >> --- a/io/pwrite.c
> >> +++ b/io/pwrite.c
> >> @@ -52,6 +52,9 @@ pwrite_help(void)
> >>  "         (heh, zorry, the -s/-S arguments were already in use in pwrite)\n"
> >>  #ifdef HAVE_PWRITEV
> >>  " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n"
> >> +#ifdef HAVE_PWRITEV2
> >> +" -N   -- Perform the pwritev2() with RWF_NOWAIT\n"
> >> +#endif
> > 
> > Separate these two? i.e.
> > 
> > #ifdef HAVE_PWRITEV
> > " -V N..."
> > #endif
> > #ifdef HAVE_PWRITEV2
> > " -N ..."
> > #endif
> > 
> > They're tested separately in configure, so the ifdefs needn't be nested.
> > 
> >>  #endif
> >>  "\n"));
> >>  }
> >> @@ -276,7 +279,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);
> >> @@ -305,6 +308,9 @@ pwrite_f(
> >>  		case 'i':
> >>  			infile = optarg;
> >>  			break;
> >> +		case 'N':
> >> +			pwritev2_flags |= RWF_NOWAIT;
> >> +			break;
> > 
> > Needs #ifdef HAVE_PWRITEV2.
> > 
> > If you feel the need to be extra cautious, you could also put:
> > 
> > #ifndef HAVE_PWRITEV2
> > 	assert(pwritev2_flags == 0);
> > #endif
> > 
> > ...just after the getopt parsing to make sure that we never silently
> > drop RWF_* flags due to code bugs.
> > 
> 
> Instead I am planning to put an #else to #ifdef in the case 'N' to make
> sure that people who do not have pwritev2() get the proper message
> instead of the command failing without reason.
> 
> +               case 'N':
> +#ifdef HAVE_PWRITEV2
> +                       pwritev2_flags |= RWF_NOWAIT;
> +                       break;
> +#else
> +                       printf(_("-N: Kernel does not support
> pwritev2()\n"));
> +                       return 0;
> +#endif

Why add support for a command only to say "command not supported"?
The error message is also incorrect - the build environment didn't
support pwritev2, not the kernel the xfs_io binary is currently
running on.

As it is, I really don't like this sort of ifdef pattern because it
means over time we'll end up with an ifdef mess in the option
parsing as more flags are added. Just make the default case say
"command -%c not supported", and that removes the need for any of
these else cases.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[Goldwyn Rodrigues]

On 10/09/2017 05:37 PM, Dave Chinner wrote:
> On Mon, Oct 09, 2017 at 04:11:17PM -0500, Goldwyn Rodrigues wrote:
>>
>>
>> On 10/09/2017 12:19 PM, Darrick J. Wong wrote:
>>> On Fri, Sep 29, 2017 at 08:00:34AM -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>
>>>> ---
>>>>  io/pwrite.c       | 8 +++++++-
>>>>  man/man8/xfs_io.8 | 6 ++++++
>>>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/io/pwrite.c b/io/pwrite.c
>>>> index e7d411bb..2b85a528 100644
>>>> --- a/io/pwrite.c
>>>> +++ b/io/pwrite.c
>>>> @@ -52,6 +52,9 @@ pwrite_help(void)
>>>>  "         (heh, zorry, the -s/-S arguments were already in use in pwrite)\n"
>>>>  #ifdef HAVE_PWRITEV
>>>>  " -V N -- use vectored IO with N iovecs of blocksize each (pwritev)\n"
>>>> +#ifdef HAVE_PWRITEV2
>>>> +" -N   -- Perform the pwritev2() with RWF_NOWAIT\n"
>>>> +#endif
>>>
>>> Separate these two? i.e.
>>>
>>> #ifdef HAVE_PWRITEV
>>> " -V N..."
>>> #endif
>>> #ifdef HAVE_PWRITEV2
>>> " -N ..."
>>> #endif
>>>
>>> They're tested separately in configure, so the ifdefs needn't be nested.
>>>
>>>>  #endif
>>>>  "\n"));
>>>>  }
>>>> @@ -276,7 +279,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);
>>>> @@ -305,6 +308,9 @@ pwrite_f(
>>>>  		case 'i':
>>>>  			infile = optarg;
>>>>  			break;
>>>> +		case 'N':
>>>> +			pwritev2_flags |= RWF_NOWAIT;
>>>> +			break;
>>>
>>> Needs #ifdef HAVE_PWRITEV2.
>>>
>>> If you feel the need to be extra cautious, you could also put:
>>>
>>> #ifndef HAVE_PWRITEV2
>>> 	assert(pwritev2_flags == 0);
>>> #endif
>>>
>>> ...just after the getopt parsing to make sure that we never silently
>>> drop RWF_* flags due to code bugs.
>>>
>>
>> Instead I am planning to put an #else to #ifdef in the case 'N' to make
>> sure that people who do not have pwritev2() get the proper message
>> instead of the command failing without reason.
>>
>> +               case 'N':
>> +#ifdef HAVE_PWRITEV2
>> +                       pwritev2_flags |= RWF_NOWAIT;
>> +                       break;
>> +#else
>> +                       printf(_("-N: Kernel does not support
>> pwritev2()\n"));
>> +                       return 0;
>> +#endif
> 
> Why add support for a command only to say "command not supported"?
> The error message is also incorrect - the build environment didn't
> support pwritev2, not the kernel the xfs_io binary is currently
> running on.
> 
> As it is, I really don't like this sort of ifdef pattern because it
> means over time we'll end up with an ifdef mess in the option
> parsing as more flags are added. Just make the default case say
> "command -%c not supported", and that removes the need for any of
> these else cases.
> 

In that case, it would go to "default" which will print the
command_usage. I am fine with that as long as the user understands why
-N is not working.

-- 
Goldwyn