All of lore.kernel.org
 help / color / mirror / Atom feed
* [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing
@ 2017-12-05 23:56 ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests

This is the second revision of my MAP_SYNC + dm-log-writes support for
xfsprogs.  The previous revision can be found here:

https://lists.01.org/pipermail/linux-nvdimm/2017-November/013326.html

Changes since v1:

 - Updated the dm-log-writes support so that it uses libdevmapper
   instead of calling the "dmsetup" stand-alone exectuable via system().
   (Eric and Darrick)

 - Fixed our MAP_SYNC handling so that instead of defining the flags for
   systems that don't have them in the headers, just set them to 0 and
   fail when the -S flag is used. (Dan)

You can find an xfsprogs branch with this series here:

https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/log/?h=map_sync_v2

Both MAP_SYNC and the DAX enhancements for dm-log-writes can be found in
v4.15-rc*.  For ease of testing I've posted a kernel that is v4.14 plus
just those two patch series here:

https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/linux.git/log/?h=map_sync_dm_log_writes

---

As suggested by Dave Chinner:

    As I say to all these sorts of one-off test prgrams: please add the
    new MAP_SYNC flag to xfs_io rather than writing a one-off
    test program to set it and write some data.

    And if we're going to be adding special custom tests just because
    we need to insert dm-log marks, add that functionality to xfs_io,
    too.

    That way we can create complex custom dm logwrite tests without
    needing one-off test programs for them all...

This series enhances xfs_io by adding support for the MAP_SYNC mmap() flag
and for dm-log-writes marks.  This allows the resulting xfstest for
MAP_SYNC to be much simpler and have no custom C programs.

Ross Zwisler (3):
  xfs_io: fix compiler warnings in getfsmap code
  xfs_io: add MAP_SYNC support to mmap()
  xfs_io: add a new 'log_writes' command

 configure.ac            |   2 +
 debian/control          |   2 +-
 include/builddefs.in    |   3 ++
 include/linux.h         |   8 ++++
 io/Makefile             |  10 +++++
 io/fsmap.c              |   4 +-
 io/init.c               |   1 +
 io/io.h                 |   7 ++++
 io/log_writes.c         | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
 io/mmap.c               |  23 ++++++++---
 m4/Makefile             |   1 +
 m4/package_devmapper.m4 |  11 ++++++
 m4/package_libcdev.m4   |  16 ++++++++
 man/man8/xfs_io.8       |  29 +++++++++++++-
 14 files changed, 208 insertions(+), 10 deletions(-)
 create mode 100644 io/log_writes.c
 create mode 100644 m4/package_devmapper.m4

-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing
@ 2017-12-05 23:56 ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs
  Cc: Ross Zwisler, linux-nvdimm, fstests, Jan Kara, Dave Chinner,
	Dan Williams

This is the second revision of my MAP_SYNC + dm-log-writes support for
xfsprogs.  The previous revision can be found here:

https://lists.01.org/pipermail/linux-nvdimm/2017-November/013326.html

Changes since v1:

 - Updated the dm-log-writes support so that it uses libdevmapper
   instead of calling the "dmsetup" stand-alone exectuable via system().
   (Eric and Darrick)

 - Fixed our MAP_SYNC handling so that instead of defining the flags for
   systems that don't have them in the headers, just set them to 0 and
   fail when the -S flag is used. (Dan)

You can find an xfsprogs branch with this series here:

https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/log/?h=map_sync_v2

Both MAP_SYNC and the DAX enhancements for dm-log-writes can be found in
v4.15-rc*.  For ease of testing I've posted a kernel that is v4.14 plus
just those two patch series here:

https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/linux.git/log/?h=map_sync_dm_log_writes

---

As suggested by Dave Chinner:

    As I say to all these sorts of one-off test prgrams: please add the
    new MAP_SYNC flag to xfs_io rather than writing a one-off
    test program to set it and write some data.

    And if we're going to be adding special custom tests just because
    we need to insert dm-log marks, add that functionality to xfs_io,
    too.

    That way we can create complex custom dm logwrite tests without
    needing one-off test programs for them all...

This series enhances xfs_io by adding support for the MAP_SYNC mmap() flag
and for dm-log-writes marks.  This allows the resulting xfstest for
MAP_SYNC to be much simpler and have no custom C programs.

Ross Zwisler (3):
  xfs_io: fix compiler warnings in getfsmap code
  xfs_io: add MAP_SYNC support to mmap()
  xfs_io: add a new 'log_writes' command

 configure.ac            |   2 +
 debian/control          |   2 +-
 include/builddefs.in    |   3 ++
 include/linux.h         |   8 ++++
 io/Makefile             |  10 +++++
 io/fsmap.c              |   4 +-
 io/init.c               |   1 +
 io/io.h                 |   7 ++++
 io/log_writes.c         | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
 io/mmap.c               |  23 ++++++++---
 m4/Makefile             |   1 +
 m4/package_devmapper.m4 |  11 ++++++
 m4/package_libcdev.m4   |  16 ++++++++
 man/man8/xfs_io.8       |  29 +++++++++++++-
 14 files changed, 208 insertions(+), 10 deletions(-)
 create mode 100644 io/log_writes.c
 create mode 100644 m4/package_devmapper.m4

-- 
2.14.3


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
  2017-12-05 23:56 ` Ross Zwisler
@ 2017-12-05 23:56   ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs; +Cc: Jan Kara, linux-nvdimm, Darrick J . Wong, Dave Chinner, fstests

I recently upgraded my compiler from
	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
to
	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
and started getting a bunch of compiler warnings in io/fsmap.c:

  fsmap.c: In function ‘fsmap_f’:
  fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
  between 1 and 17 bytes into a region of size between 12 and 28
  [-Wformat-truncation=]
     snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
  ^~~~
  fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
     snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
  ^~~~~~~~~~~~~~~
  fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
  destination of size 32
     snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  (long long)BTOBBT(p->fmr_physical),
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The issue is that 'bbuf' is only defined to be 32 characters wide, but each
signed long long can potentially print as many as 19 characters
(9223372036854775807 is the max value).  The format we're using for bbuf is
"[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
"[..]:\0", which means it's possible we'll print up to 44 characters,
overflowing our 32 char buffer.

Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
characters.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Darrick J. Wong <darrick.wong@oracle.com>
Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
---
 io/fsmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io/fsmap.c b/io/fsmap.c
index 448fb535..3d8a6700 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -184,8 +184,8 @@ dump_map_verbose(
 	off64_t			agoff, bperag;
 	int			foff_w, boff_w, aoff_w, tot_w, agno_w, own_w;
 	int			nr_w, dev_w;
-	char			rbuf[32], bbuf[32], abuf[32], obuf[32];
-	char			nbuf[32], dbuf[32], gbuf[32];
+	char			rbuf[64], bbuf[64], abuf[64], obuf[64];
+	char			nbuf[64], dbuf[64], gbuf[64];
 	char			owner[OWNER_BUF_SZ];
 	int			sunit, swidth;
 	int			flg = 0;
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
@ 2017-12-05 23:56   ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs
  Cc: Ross Zwisler, linux-nvdimm, fstests, Jan Kara, Dave Chinner,
	Dan Williams, Darrick J . Wong

I recently upgraded my compiler from
	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
to
	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
and started getting a bunch of compiler warnings in io/fsmap.c:

  fsmap.c: In function ‘fsmap_f’:
  fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
  between 1 and 17 bytes into a region of size between 12 and 28
  [-Wformat-truncation=]
     snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
  ^~~~
  fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
     snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
  ^~~~~~~~~~~~~~~
  fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
  destination of size 32
     snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  (long long)BTOBBT(p->fmr_physical),
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The issue is that 'bbuf' is only defined to be 32 characters wide, but each
signed long long can potentially print as many as 19 characters
(9223372036854775807 is the max value).  The format we're using for bbuf is
"[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
"[..]:\0", which means it's possible we'll print up to 44 characters,
overflowing our 32 char buffer.

Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
characters.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Darrick J. Wong <darrick.wong@oracle.com>
Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
---
 io/fsmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io/fsmap.c b/io/fsmap.c
index 448fb535..3d8a6700 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -184,8 +184,8 @@ dump_map_verbose(
 	off64_t			agoff, bperag;
 	int			foff_w, boff_w, aoff_w, tot_w, agno_w, own_w;
 	int			nr_w, dev_w;
-	char			rbuf[32], bbuf[32], abuf[32], obuf[32];
-	char			nbuf[32], dbuf[32], gbuf[32];
+	char			rbuf[64], bbuf[64], abuf[64], obuf[64];
+	char			nbuf[64], dbuf[64], gbuf[64];
 	char			owner[OWNER_BUF_SZ];
 	int			sunit, swidth;
 	int			flg = 0;
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
  2017-12-05 23:56 ` Ross Zwisler
@ 2017-12-05 23:56   ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests

Add support for a new -S flag to xfs_io's mmap command.  This opens the
mapping with the (MAP_SYNC | MAP_SHARED_VALIDATE) flags instead of the
standard MAP_SHARED flag.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
---
 configure.ac          |  1 +
 include/builddefs.in  |  1 +
 include/linux.h       |  8 ++++++++
 io/Makefile           |  4 ++++
 io/io.h               |  1 +
 io/mmap.c             | 23 ++++++++++++++++++-----
 m4/package_libcdev.m4 | 16 ++++++++++++++++
 man/man8/xfs_io.8     |  6 +++++-
 8 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index e5d0309f..f3325aa0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -163,6 +163,7 @@ AC_HAVE_MREMAP
 AC_NEED_INTERNAL_FSXATTR
 AC_HAVE_GETFSMAP
 AC_HAVE_STATFS_FLAGS
+AC_HAVE_MAP_SYNC
 
 if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
diff --git a/include/builddefs.in b/include/builddefs.in
index fd274ddc..126f7e95 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -115,6 +115,7 @@ HAVE_MREMAP = @have_mremap@
 NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
 HAVE_GETFSMAP = @have_getfsmap@
 HAVE_STATFS_FLAGS = @have_statfs_flags@
+HAVE_MAP_SYNC = @have_map_sync@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
diff --git a/include/linux.h b/include/linux.h
index 6ce344c5..1998941a 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -327,4 +327,12 @@ fsmap_advance(
 #define HAVE_GETFSMAP
 #endif /* HAVE_GETFSMAP */
 
+#ifndef HAVE_MAP_SYNC
+#define MAP_SYNC 0
+#define MAP_SHARED_VALIDATE 0
+#else
+#include <asm-generic/mman.h>
+#include <asm-generic/mman-common.h>
+#endif /* HAVE_MAP_SYNC */
+
 #endif	/* __XFS_LINUX_H__ */
diff --git a/io/Makefile b/io/Makefile
index 6725936d..2987ee11 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -103,6 +103,10 @@ ifeq ($(HAVE_MREMAP),yes)
 LCFLAGS += -DHAVE_MREMAP
 endif
 
+ifeq ($(HAVE_MAP_SYNC),yes)
+LCFLAGS += -DHAVE_MAP_SYNC
+endif
+
 # On linux we get fsmap from the system or define it ourselves
 # so include this based on platform type.  If this reverts to only
 # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
diff --git a/io/io.h b/io/io.h
index 3862985f..8b2753b3 100644
--- a/io/io.h
+++ b/io/io.h
@@ -65,6 +65,7 @@ typedef struct mmap_region {
 	size_t		length;		/* length of mapping */
 	off64_t		offset;		/* start offset into backing file */
 	int		prot;		/* protection mode of the mapping */
+	bool		map_sync;	/* is this a MAP_SYNC mapping? */
 	char		*name;		/* name of backing file */
 } mmap_region_t;
 
diff --git a/io/mmap.c b/io/mmap.c
index 7a8150e3..84319f48 100644
--- a/io/mmap.c
+++ b/io/mmap.c
@@ -42,7 +42,7 @@ print_mapping(
 	int		index,
 	int		braces)
 {
-	unsigned char	buffer[8] = { 0 };
+	char		buffer[8] = { 0 };
 	int		i;
 
 	static struct {
@@ -57,6 +57,10 @@ print_mapping(
 
 	for (i = 0, p = pflags; p->prot != PROT_NONE; i++, p++)
 		buffer[i] = (map->prot & p->prot) ? p->mode : '-';
+
+	if (map->map_sync)
+		sprintf(&buffer[i], " S");
+
 	printf("%c%03d%c 0x%lx - 0x%lx %s  %14s (%lld : %ld)\n",
 		braces? '[' : ' ', index, braces? ']' : ' ',
 		(unsigned long)map->addr,
@@ -146,6 +150,7 @@ mmap_help(void)
 " -r -- map with PROT_READ protection\n"
 " -w -- map with PROT_WRITE protection\n"
 " -x -- map with PROT_EXEC protection\n"
+" -S -- map with MAP_SYNC and MAP_SHARED_VALIDATE flags\n"
 " -s <size> -- first do mmap(size)/munmap(size), try to reserve some free space\n"
 " If no protection mode is specified, all are used by default.\n"
 "\n"));
@@ -161,7 +166,7 @@ mmap_f(
 	void		*address = NULL;
 	char		*filename;
 	size_t		blocksize, sectsize;
-	int		c, prot = 0;
+	int		c, prot = 0, flags = MAP_SHARED;
 
 	if (argc == 1) {
 		if (mapping)
@@ -184,7 +189,7 @@ mmap_f(
 
 	init_cvtnum(&blocksize, &sectsize);
 
-	while ((c = getopt(argc, argv, "rwxs:")) != EOF) {
+	while ((c = getopt(argc, argv, "rwxSs:")) != EOF) {
 		switch (c) {
 		case 'r':
 			prot |= PROT_READ;
@@ -195,6 +200,13 @@ mmap_f(
 		case 'x':
 			prot |= PROT_EXEC;
 			break;
+		case 'S':
+			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
+			if (!flags) {
+				printf("MAP_SYNC not supported\n");
+				return 0;
+			}
+			break;
 		case 's':
 			length2 = cvtnum(blocksize, sectsize, optarg);
 			break;
@@ -238,7 +250,7 @@ mmap_f(
 		               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 		munmap(address, length2);
 	}
-	address = mmap(address, length, prot, MAP_SHARED, file->fd, offset);
+	address = mmap(address, length, prot, flags, file->fd, offset);
 	if (address == MAP_FAILED) {
 		perror("mmap");
 		free(filename);
@@ -263,6 +275,7 @@ mmap_f(
 	mapping->offset = offset;
 	mapping->name = filename;
 	mapping->prot = prot;
+	mapping->map_sync = (flags == (MAP_SYNC | MAP_SHARED_VALIDATE));
 	return 0;
 }
 
@@ -676,7 +689,7 @@ mmap_init(void)
 	mmap_cmd.argmax = -1;
 	mmap_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
 			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
-	mmap_cmd.args = _("[N] | [-rwx] [-s size] [off len]");
+	mmap_cmd.args = _("[N] | [-rwxS] [-s size] [off len]");
 	mmap_cmd.oneline =
 		_("mmap a range in the current file, show mappings");
 	mmap_cmd.help = mmap_help;
diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
index 7b4dfc85..71cedc5c 100644
--- a/m4/package_libcdev.m4
+++ b/m4/package_libcdev.m4
@@ -328,3 +328,19 @@ AC_DEFUN([AC_HAVE_STATFS_FLAGS],
     )
     AC_SUBST(have_statfs_flags)
   ])
+
+#
+# Check if we have MAP_SYNC defines (Linux)
+#
+AC_DEFUN([AC_HAVE_MAP_SYNC],
+  [ AC_MSG_CHECKING([for MAP_SYNC])
+    AC_TRY_COMPILE([
+#include <asm-generic/mman.h>
+#include <asm-generic/mman-common.h>
+    ], [
+        int flags = MAP_SYNC | MAP_SHARED_VALIDATE;
+    ], have_map_sync=yes
+	AC_MSG_RESULT(yes),
+	AC_MSG_RESULT(no))
+    AC_SUBST(have_map_sync)
+  ])
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 9bf1a478..1693f7f1 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
 
 .SH MEMORY MAPPED I/O COMMANDS
 .TP
-.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
+.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
 With no arguments,
 .B mmap
 shows the current mappings. Specifying a single numeric argument
@@ -780,6 +780,10 @@ PROT_WRITE
 .RB ( \-w ),
 and PROT_EXEC
 .RB ( \-x ).
+The mapping will be created with the MAP_SHARED flag by default, or with the
+Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if
+.B -S
+is given.
 .BI \-s " size"
 is used to do a mmap(size) && munmap(size) operation at first, try to reserve some
 extendible free memory space, if
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
@ 2017-12-05 23:56   ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs
  Cc: Ross Zwisler, linux-nvdimm, fstests, Jan Kara, Dave Chinner,
	Dan Williams

Add support for a new -S flag to xfs_io's mmap command.  This opens the
mapping with the (MAP_SYNC | MAP_SHARED_VALIDATE) flags instead of the
standard MAP_SHARED flag.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
---
 configure.ac          |  1 +
 include/builddefs.in  |  1 +
 include/linux.h       |  8 ++++++++
 io/Makefile           |  4 ++++
 io/io.h               |  1 +
 io/mmap.c             | 23 ++++++++++++++++++-----
 m4/package_libcdev.m4 | 16 ++++++++++++++++
 man/man8/xfs_io.8     |  6 +++++-
 8 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index e5d0309f..f3325aa0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -163,6 +163,7 @@ AC_HAVE_MREMAP
 AC_NEED_INTERNAL_FSXATTR
 AC_HAVE_GETFSMAP
 AC_HAVE_STATFS_FLAGS
+AC_HAVE_MAP_SYNC
 
 if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
diff --git a/include/builddefs.in b/include/builddefs.in
index fd274ddc..126f7e95 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -115,6 +115,7 @@ HAVE_MREMAP = @have_mremap@
 NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
 HAVE_GETFSMAP = @have_getfsmap@
 HAVE_STATFS_FLAGS = @have_statfs_flags@
+HAVE_MAP_SYNC = @have_map_sync@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
diff --git a/include/linux.h b/include/linux.h
index 6ce344c5..1998941a 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -327,4 +327,12 @@ fsmap_advance(
 #define HAVE_GETFSMAP
 #endif /* HAVE_GETFSMAP */
 
+#ifndef HAVE_MAP_SYNC
+#define MAP_SYNC 0
+#define MAP_SHARED_VALIDATE 0
+#else
+#include <asm-generic/mman.h>
+#include <asm-generic/mman-common.h>
+#endif /* HAVE_MAP_SYNC */
+
 #endif	/* __XFS_LINUX_H__ */
diff --git a/io/Makefile b/io/Makefile
index 6725936d..2987ee11 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -103,6 +103,10 @@ ifeq ($(HAVE_MREMAP),yes)
 LCFLAGS += -DHAVE_MREMAP
 endif
 
+ifeq ($(HAVE_MAP_SYNC),yes)
+LCFLAGS += -DHAVE_MAP_SYNC
+endif
+
 # On linux we get fsmap from the system or define it ourselves
 # so include this based on platform type.  If this reverts to only
 # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
diff --git a/io/io.h b/io/io.h
index 3862985f..8b2753b3 100644
--- a/io/io.h
+++ b/io/io.h
@@ -65,6 +65,7 @@ typedef struct mmap_region {
 	size_t		length;		/* length of mapping */
 	off64_t		offset;		/* start offset into backing file */
 	int		prot;		/* protection mode of the mapping */
+	bool		map_sync;	/* is this a MAP_SYNC mapping? */
 	char		*name;		/* name of backing file */
 } mmap_region_t;
 
diff --git a/io/mmap.c b/io/mmap.c
index 7a8150e3..84319f48 100644
--- a/io/mmap.c
+++ b/io/mmap.c
@@ -42,7 +42,7 @@ print_mapping(
 	int		index,
 	int		braces)
 {
-	unsigned char	buffer[8] = { 0 };
+	char		buffer[8] = { 0 };
 	int		i;
 
 	static struct {
@@ -57,6 +57,10 @@ print_mapping(
 
 	for (i = 0, p = pflags; p->prot != PROT_NONE; i++, p++)
 		buffer[i] = (map->prot & p->prot) ? p->mode : '-';
+
+	if (map->map_sync)
+		sprintf(&buffer[i], " S");
+
 	printf("%c%03d%c 0x%lx - 0x%lx %s  %14s (%lld : %ld)\n",
 		braces? '[' : ' ', index, braces? ']' : ' ',
 		(unsigned long)map->addr,
@@ -146,6 +150,7 @@ mmap_help(void)
 " -r -- map with PROT_READ protection\n"
 " -w -- map with PROT_WRITE protection\n"
 " -x -- map with PROT_EXEC protection\n"
+" -S -- map with MAP_SYNC and MAP_SHARED_VALIDATE flags\n"
 " -s <size> -- first do mmap(size)/munmap(size), try to reserve some free space\n"
 " If no protection mode is specified, all are used by default.\n"
 "\n"));
@@ -161,7 +166,7 @@ mmap_f(
 	void		*address = NULL;
 	char		*filename;
 	size_t		blocksize, sectsize;
-	int		c, prot = 0;
+	int		c, prot = 0, flags = MAP_SHARED;
 
 	if (argc == 1) {
 		if (mapping)
@@ -184,7 +189,7 @@ mmap_f(
 
 	init_cvtnum(&blocksize, &sectsize);
 
-	while ((c = getopt(argc, argv, "rwxs:")) != EOF) {
+	while ((c = getopt(argc, argv, "rwxSs:")) != EOF) {
 		switch (c) {
 		case 'r':
 			prot |= PROT_READ;
@@ -195,6 +200,13 @@ mmap_f(
 		case 'x':
 			prot |= PROT_EXEC;
 			break;
+		case 'S':
+			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
+			if (!flags) {
+				printf("MAP_SYNC not supported\n");
+				return 0;
+			}
+			break;
 		case 's':
 			length2 = cvtnum(blocksize, sectsize, optarg);
 			break;
@@ -238,7 +250,7 @@ mmap_f(
 		               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 		munmap(address, length2);
 	}
-	address = mmap(address, length, prot, MAP_SHARED, file->fd, offset);
+	address = mmap(address, length, prot, flags, file->fd, offset);
 	if (address == MAP_FAILED) {
 		perror("mmap");
 		free(filename);
@@ -263,6 +275,7 @@ mmap_f(
 	mapping->offset = offset;
 	mapping->name = filename;
 	mapping->prot = prot;
+	mapping->map_sync = (flags == (MAP_SYNC | MAP_SHARED_VALIDATE));
 	return 0;
 }
 
@@ -676,7 +689,7 @@ mmap_init(void)
 	mmap_cmd.argmax = -1;
 	mmap_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
 			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
-	mmap_cmd.args = _("[N] | [-rwx] [-s size] [off len]");
+	mmap_cmd.args = _("[N] | [-rwxS] [-s size] [off len]");
 	mmap_cmd.oneline =
 		_("mmap a range in the current file, show mappings");
 	mmap_cmd.help = mmap_help;
diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
index 7b4dfc85..71cedc5c 100644
--- a/m4/package_libcdev.m4
+++ b/m4/package_libcdev.m4
@@ -328,3 +328,19 @@ AC_DEFUN([AC_HAVE_STATFS_FLAGS],
     )
     AC_SUBST(have_statfs_flags)
   ])
+
+#
+# Check if we have MAP_SYNC defines (Linux)
+#
+AC_DEFUN([AC_HAVE_MAP_SYNC],
+  [ AC_MSG_CHECKING([for MAP_SYNC])
+    AC_TRY_COMPILE([
+#include <asm-generic/mman.h>
+#include <asm-generic/mman-common.h>
+    ], [
+        int flags = MAP_SYNC | MAP_SHARED_VALIDATE;
+    ], have_map_sync=yes
+	AC_MSG_RESULT(yes),
+	AC_MSG_RESULT(no))
+    AC_SUBST(have_map_sync)
+  ])
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 9bf1a478..1693f7f1 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
 
 .SH MEMORY MAPPED I/O COMMANDS
 .TP
-.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
+.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
 With no arguments,
 .B mmap
 shows the current mappings. Specifying a single numeric argument
@@ -780,6 +780,10 @@ PROT_WRITE
 .RB ( \-w ),
 and PROT_EXEC
 .RB ( \-x ).
+The mapping will be created with the MAP_SHARED flag by default, or with the
+Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if
+.B -S
+is given.
 .BI \-s " size"
 is used to do a mmap(size) && munmap(size) operation at first, try to reserve some
 extendible free memory space, if
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
  2017-12-05 23:56 ` Ross Zwisler
@ 2017-12-05 23:56   ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests

Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
log marks.  It's helpful to allow users of xfs_io to adds these marks from
within xfs_io instead of waiting until after xfs_io exits because then they
are able to replay the dm-log-writes log up to immediately after another
xfs_io operation such as mwrite.  This isolates the log replay from other
operations that happen as part of xfs_io exiting (file handles being
closed, mmaps being torn down, etc.).  This also allows users to insert
multiple marks between different xfs_io commands.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
---
 configure.ac            |   1 +
 debian/control          |   2 +-
 include/builddefs.in    |   2 +
 io/Makefile             |   6 +++
 io/init.c               |   1 +
 io/io.h                 |   6 +++
 io/log_writes.c         | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
 m4/Makefile             |   1 +
 m4/package_devmapper.m4 |  11 ++++++
 man/man8/xfs_io.8       |  23 ++++++++++-
 10 files changed, 152 insertions(+), 2 deletions(-)
 create mode 100644 io/log_writes.c
 create mode 100644 m4/package_devmapper.m4

diff --git a/configure.ac b/configure.ac
index f3325aa0..f83d5817 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,6 +164,7 @@ AC_NEED_INTERNAL_FSXATTR
 AC_HAVE_GETFSMAP
 AC_HAVE_STATFS_FLAGS
 AC_HAVE_MAP_SYNC
+AC_HAVE_DEVMAPPER
 
 if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
diff --git a/debian/control b/debian/control
index ad816622..5c26e3d7 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
-Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev
+Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev
 Standards-Version: 3.9.1
 Homepage: http://xfs.org/
 
diff --git a/include/builddefs.in b/include/builddefs.in
index 126f7e95..66bdbfa2 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -35,6 +35,7 @@ LIBTERMCAP = @libtermcap@
 LIBEDITLINE = @libeditline@
 LIBREADLINE = @libreadline@
 LIBBLKID = @libblkid@
+LIBDEVMAPPER = @libdevmapper@
 LIBXFS = $(TOPDIR)/libxfs/libxfs.la
 LIBXCMD = $(TOPDIR)/libxcmd/libxcmd.la
 LIBXLOG = $(TOPDIR)/libxlog/libxlog.la
@@ -116,6 +117,7 @@ NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
 HAVE_GETFSMAP = @have_getfsmap@
 HAVE_STATFS_FLAGS = @have_statfs_flags@
 HAVE_MAP_SYNC = @have_map_sync@
+HAVE_DEVMAPPER = @have_devmapper@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
diff --git a/io/Makefile b/io/Makefile
index 2987ee11..b7585a1b 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -107,6 +107,12 @@ ifeq ($(HAVE_MAP_SYNC),yes)
 LCFLAGS += -DHAVE_MAP_SYNC
 endif
 
+ifeq ($(HAVE_DEVMAPPER),yes)
+CFILES += log_writes.c
+LLDLIBS += $(LIBDEVMAPPER)
+LCFLAGS += -DHAVE_DEVMAPPER
+endif
+
 # On linux we get fsmap from the system or define it ourselves
 # so include this based on platform type.  If this reverts to only
 # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
diff --git a/io/init.c b/io/init.c
index 20d5f80d..34d87b5f 100644
--- a/io/init.c
+++ b/io/init.c
@@ -72,6 +72,7 @@ init_commands(void)
 	help_init();
 	imap_init();
 	inject_init();
+	log_writes_init();
 	madvise_init();
 	mincore_init();
 	mmap_init();
diff --git a/io/io.h b/io/io.h
index 8b2753b3..9349cc75 100644
--- a/io/io.h
+++ b/io/io.h
@@ -186,3 +186,9 @@ extern void		fsmap_init(void);
 #else
 # define fsmap_init()	do { } while (0)
 #endif
+
+#ifdef HAVE_DEVMAPPER
+extern void		log_writes_init(void);
+#else
+#define log_writes_init()	do { } while (0)
+#endif
diff --git a/io/log_writes.c b/io/log_writes.c
new file mode 100644
index 00000000..37c0024f
--- /dev/null
+++ b/io/log_writes.c
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "platform_defs.h"
+#include <libdevmapper.h>
+#include "command.h"
+#include "init.h"
+
+static cmdinfo_t log_writes_cmd;
+
+static void
+mark_log(const char *device, const char *mark)
+{
+	struct dm_task *dmt;
+	const int size = 80;
+	char message[size];
+	int len;
+
+	len = snprintf(message, size, "mark %s", mark);
+	if (len >= size) {
+		printf("mark '%s' is too long\n", mark);
+		return;
+	}
+
+	if (!(dmt = dm_task_create(DM_DEVICE_TARGET_MSG)))
+		return;
+
+	if (!dm_task_set_name(dmt, device))
+		goto out;
+
+	if (!dm_task_set_sector(dmt, 0))
+		goto out;
+
+	if (!dm_task_set_message(dmt, message))
+		goto out;
+
+	dm_task_run(dmt);
+out:
+	dm_task_destroy(dmt);
+}
+
+static int
+log_writes_f(
+	int			argc,
+	char			**argv)
+{
+	const char *device = NULL;
+	const char *mark = NULL;
+	int c;
+
+	while ((c = getopt(argc, argv, "d:m:")) != EOF) {
+		switch (c) {
+		case 'd':
+			device = optarg;
+			break;
+		case 'm':
+			mark = optarg;
+			break;
+		default:
+			return command_usage(&log_writes_cmd);
+		}
+	}
+
+	if (device == NULL || mark == NULL)
+		return command_usage(&log_writes_cmd);
+
+	mark_log(device, mark);
+	return 0;
+}
+
+void
+log_writes_init(void)
+{
+	log_writes_cmd.name = "log_writes";
+	log_writes_cmd.altname = "lw";
+	log_writes_cmd.cfunc = log_writes_f;
+	log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK
+				| CMD_FLAG_ONESHOT;
+	log_writes_cmd.argmin = 0;
+	log_writes_cmd.argmax = -1;
+	log_writes_cmd.args = _("-d device -m mark");
+	log_writes_cmd.oneline =
+		_("create mark <mark> in the dm-log-writes log specified by <device>");
+
+	add_command(&log_writes_cmd);
+}
diff --git a/m4/Makefile b/m4/Makefile
index 4706121b..77f2edda 100644
--- a/m4/Makefile
+++ b/m4/Makefile
@@ -15,6 +15,7 @@ CONFIGURE = \
 LSRCFILES = \
 	manual_format.m4 \
 	package_blkid.m4 \
+	package_devmapper.m4 \
 	package_globals.m4 \
 	package_libcdev.m4 \
 	package_pthread.m4 \
diff --git a/m4/package_devmapper.m4 b/m4/package_devmapper.m4
new file mode 100644
index 00000000..821f283c
--- /dev/null
+++ b/m4/package_devmapper.m4
@@ -0,0 +1,11 @@
+#
+# See if libdevmapper is available on the system.
+#
+AC_DEFUN([AC_HAVE_DEVMAPPER],
+[ AC_SEARCH_LIBS([dm_task_create], [devmapper],
+        libdevmapper="-ldevmapper"
+        have_devmapper=yes,
+        have_devmapper=no,)
+    AC_SUBST(have_devmapper)
+    AC_SUBST(libdevmapper)
+])
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 1693f7f1..4fce6d39 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1123,7 +1123,27 @@ version of policy structure (numeric)
 .BR get_encpolicy
 On filesystems that support encryption, display the encryption policy of the
 current file.
+.RE
+.PD
+.TP
+.BI "log_writes \-d " device " \-m "  mark
+Create a mark named
+.I mark
+in the dm-log-writes log specified by
+.I device.
+This is intended to be equivalent to the shell command:
 
+.B dmsetup message
+.I device
+.B 0 mark
+.I mark
+.PD
+.RE
+.TP
+.B lw
+See the
+.B log_writes
+command.
 .SH SEE ALSO
 .BR mkfs.xfs (8),
 .BR xfsctl (3),
@@ -1141,4 +1161,5 @@ current file.
 .BR open (2),
 .BR pread (2),
 .BR pwrite (2),
-.BR readdir (3).
+.BR readdir (3),
+.BR dmsetup (8).
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
@ 2017-12-05 23:56   ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-05 23:56 UTC (permalink / raw)
  To: linux-xfs
  Cc: Ross Zwisler, linux-nvdimm, fstests, Jan Kara, Dave Chinner,
	Dan Williams

Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
log marks.  It's helpful to allow users of xfs_io to adds these marks from
within xfs_io instead of waiting until after xfs_io exits because then they
are able to replay the dm-log-writes log up to immediately after another
xfs_io operation such as mwrite.  This isolates the log replay from other
operations that happen as part of xfs_io exiting (file handles being
closed, mmaps being torn down, etc.).  This also allows users to insert
multiple marks between different xfs_io commands.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
---
 configure.ac            |   1 +
 debian/control          |   2 +-
 include/builddefs.in    |   2 +
 io/Makefile             |   6 +++
 io/init.c               |   1 +
 io/io.h                 |   6 +++
 io/log_writes.c         | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
 m4/Makefile             |   1 +
 m4/package_devmapper.m4 |  11 ++++++
 man/man8/xfs_io.8       |  23 ++++++++++-
 10 files changed, 152 insertions(+), 2 deletions(-)
 create mode 100644 io/log_writes.c
 create mode 100644 m4/package_devmapper.m4

diff --git a/configure.ac b/configure.ac
index f3325aa0..f83d5817 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,6 +164,7 @@ AC_NEED_INTERNAL_FSXATTR
 AC_HAVE_GETFSMAP
 AC_HAVE_STATFS_FLAGS
 AC_HAVE_MAP_SYNC
+AC_HAVE_DEVMAPPER
 
 if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
diff --git a/debian/control b/debian/control
index ad816622..5c26e3d7 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
-Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev
+Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev
 Standards-Version: 3.9.1
 Homepage: http://xfs.org/
 
diff --git a/include/builddefs.in b/include/builddefs.in
index 126f7e95..66bdbfa2 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -35,6 +35,7 @@ LIBTERMCAP = @libtermcap@
 LIBEDITLINE = @libeditline@
 LIBREADLINE = @libreadline@
 LIBBLKID = @libblkid@
+LIBDEVMAPPER = @libdevmapper@
 LIBXFS = $(TOPDIR)/libxfs/libxfs.la
 LIBXCMD = $(TOPDIR)/libxcmd/libxcmd.la
 LIBXLOG = $(TOPDIR)/libxlog/libxlog.la
@@ -116,6 +117,7 @@ NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
 HAVE_GETFSMAP = @have_getfsmap@
 HAVE_STATFS_FLAGS = @have_statfs_flags@
 HAVE_MAP_SYNC = @have_map_sync@
+HAVE_DEVMAPPER = @have_devmapper@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
diff --git a/io/Makefile b/io/Makefile
index 2987ee11..b7585a1b 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -107,6 +107,12 @@ ifeq ($(HAVE_MAP_SYNC),yes)
 LCFLAGS += -DHAVE_MAP_SYNC
 endif
 
+ifeq ($(HAVE_DEVMAPPER),yes)
+CFILES += log_writes.c
+LLDLIBS += $(LIBDEVMAPPER)
+LCFLAGS += -DHAVE_DEVMAPPER
+endif
+
 # On linux we get fsmap from the system or define it ourselves
 # so include this based on platform type.  If this reverts to only
 # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
diff --git a/io/init.c b/io/init.c
index 20d5f80d..34d87b5f 100644
--- a/io/init.c
+++ b/io/init.c
@@ -72,6 +72,7 @@ init_commands(void)
 	help_init();
 	imap_init();
 	inject_init();
+	log_writes_init();
 	madvise_init();
 	mincore_init();
 	mmap_init();
diff --git a/io/io.h b/io/io.h
index 8b2753b3..9349cc75 100644
--- a/io/io.h
+++ b/io/io.h
@@ -186,3 +186,9 @@ extern void		fsmap_init(void);
 #else
 # define fsmap_init()	do { } while (0)
 #endif
+
+#ifdef HAVE_DEVMAPPER
+extern void		log_writes_init(void);
+#else
+#define log_writes_init()	do { } while (0)
+#endif
diff --git a/io/log_writes.c b/io/log_writes.c
new file mode 100644
index 00000000..37c0024f
--- /dev/null
+++ b/io/log_writes.c
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "platform_defs.h"
+#include <libdevmapper.h>
+#include "command.h"
+#include "init.h"
+
+static cmdinfo_t log_writes_cmd;
+
+static void
+mark_log(const char *device, const char *mark)
+{
+	struct dm_task *dmt;
+	const int size = 80;
+	char message[size];
+	int len;
+
+	len = snprintf(message, size, "mark %s", mark);
+	if (len >= size) {
+		printf("mark '%s' is too long\n", mark);
+		return;
+	}
+
+	if (!(dmt = dm_task_create(DM_DEVICE_TARGET_MSG)))
+		return;
+
+	if (!dm_task_set_name(dmt, device))
+		goto out;
+
+	if (!dm_task_set_sector(dmt, 0))
+		goto out;
+
+	if (!dm_task_set_message(dmt, message))
+		goto out;
+
+	dm_task_run(dmt);
+out:
+	dm_task_destroy(dmt);
+}
+
+static int
+log_writes_f(
+	int			argc,
+	char			**argv)
+{
+	const char *device = NULL;
+	const char *mark = NULL;
+	int c;
+
+	while ((c = getopt(argc, argv, "d:m:")) != EOF) {
+		switch (c) {
+		case 'd':
+			device = optarg;
+			break;
+		case 'm':
+			mark = optarg;
+			break;
+		default:
+			return command_usage(&log_writes_cmd);
+		}
+	}
+
+	if (device == NULL || mark == NULL)
+		return command_usage(&log_writes_cmd);
+
+	mark_log(device, mark);
+	return 0;
+}
+
+void
+log_writes_init(void)
+{
+	log_writes_cmd.name = "log_writes";
+	log_writes_cmd.altname = "lw";
+	log_writes_cmd.cfunc = log_writes_f;
+	log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK
+				| CMD_FLAG_ONESHOT;
+	log_writes_cmd.argmin = 0;
+	log_writes_cmd.argmax = -1;
+	log_writes_cmd.args = _("-d device -m mark");
+	log_writes_cmd.oneline =
+		_("create mark <mark> in the dm-log-writes log specified by <device>");
+
+	add_command(&log_writes_cmd);
+}
diff --git a/m4/Makefile b/m4/Makefile
index 4706121b..77f2edda 100644
--- a/m4/Makefile
+++ b/m4/Makefile
@@ -15,6 +15,7 @@ CONFIGURE = \
 LSRCFILES = \
 	manual_format.m4 \
 	package_blkid.m4 \
+	package_devmapper.m4 \
 	package_globals.m4 \
 	package_libcdev.m4 \
 	package_pthread.m4 \
diff --git a/m4/package_devmapper.m4 b/m4/package_devmapper.m4
new file mode 100644
index 00000000..821f283c
--- /dev/null
+++ b/m4/package_devmapper.m4
@@ -0,0 +1,11 @@
+#
+# See if libdevmapper is available on the system.
+#
+AC_DEFUN([AC_HAVE_DEVMAPPER],
+[ AC_SEARCH_LIBS([dm_task_create], [devmapper],
+        libdevmapper="-ldevmapper"
+        have_devmapper=yes,
+        have_devmapper=no,)
+    AC_SUBST(have_devmapper)
+    AC_SUBST(libdevmapper)
+])
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 1693f7f1..4fce6d39 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1123,7 +1123,27 @@ version of policy structure (numeric)
 .BR get_encpolicy
 On filesystems that support encryption, display the encryption policy of the
 current file.
+.RE
+.PD
+.TP
+.BI "log_writes \-d " device " \-m "  mark
+Create a mark named
+.I mark
+in the dm-log-writes log specified by
+.I device.
+This is intended to be equivalent to the shell command:
 
+.B dmsetup message
+.I device
+.B 0 mark
+.I mark
+.PD
+.RE
+.TP
+.B lw
+See the
+.B log_writes
+command.
 .SH SEE ALSO
 .BR mkfs.xfs (8),
 .BR xfsctl (3),
@@ -1141,4 +1161,5 @@ current file.
 .BR open (2),
 .BR pread (2),
 .BR pwrite (2),
-.BR readdir (3).
+.BR readdir (3),
+.BR dmsetup (8).
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
  2017-12-05 23:56   ` Ross Zwisler
@ 2017-12-06  0:03     ` Darrick J. Wong
  -1 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-06  0:03 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> I recently upgraded my compiler from
> 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> to
> 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> and started getting a bunch of compiler warnings in io/fsmap.c:
> 
>   fsmap.c: In function ‘fsmap_f’:
>   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
>   between 1 and 17 bytes into a region of size between 12 and 28
>   [-Wformat-truncation=]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~
>   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~~~~~~~~~~~~
>   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
>   destination of size 32
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical),
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> signed long long can potentially print as many as 19 characters
> (9223372036854775807 is the max value).  The format we're using for bbuf is
> "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> "[..]:\0", which means it's possible we'll print up to 44 characters,
> overflowing our 32 char buffer.
> 
> Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> characters.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Darrick J. Wong <darrick.wong@oracle.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> ---
>  io/fsmap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/io/fsmap.c b/io/fsmap.c
> index 448fb535..3d8a6700 100644
> --- a/io/fsmap.c
> +++ b/io/fsmap.c
> @@ -184,8 +184,8 @@ dump_map_verbose(
>  	off64_t			agoff, bperag;
>  	int			foff_w, boff_w, aoff_w, tot_w, agno_w, own_w;
>  	int			nr_w, dev_w;
> -	char			rbuf[32], bbuf[32], abuf[32], obuf[32];
> -	char			nbuf[32], dbuf[32], gbuf[32];
> +	char			rbuf[64], bbuf[64], abuf[64], obuf[64];
> +	char			nbuf[64], dbuf[64], gbuf[64];
>  	char			owner[OWNER_BUF_SZ];
>  	int			sunit, swidth;
>  	int			flg = 0;
> -- 
> 2.14.3
> 
> --
> 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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
@ 2017-12-06  0:03     ` Darrick J. Wong
  0 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-06  0:03 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dave Chinner, Dan Williams

On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> I recently upgraded my compiler from
> 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> to
> 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> and started getting a bunch of compiler warnings in io/fsmap.c:
> 
>   fsmap.c: In function ‘fsmap_f’:
>   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
>   between 1 and 17 bytes into a region of size between 12 and 28
>   [-Wformat-truncation=]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~
>   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~~~~~~~~~~~~
>   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
>   destination of size 32
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical),
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> signed long long can potentially print as many as 19 characters
> (9223372036854775807 is the max value).  The format we're using for bbuf is
> "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> "[..]:\0", which means it's possible we'll print up to 44 characters,
> overflowing our 32 char buffer.
> 
> Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> characters.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Darrick J. Wong <darrick.wong@oracle.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> ---
>  io/fsmap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/io/fsmap.c b/io/fsmap.c
> index 448fb535..3d8a6700 100644
> --- a/io/fsmap.c
> +++ b/io/fsmap.c
> @@ -184,8 +184,8 @@ dump_map_verbose(
>  	off64_t			agoff, bperag;
>  	int			foff_w, boff_w, aoff_w, tot_w, agno_w, own_w;
>  	int			nr_w, dev_w;
> -	char			rbuf[32], bbuf[32], abuf[32], obuf[32];
> -	char			nbuf[32], dbuf[32], gbuf[32];
> +	char			rbuf[64], bbuf[64], abuf[64], obuf[64];
> +	char			nbuf[64], dbuf[64], gbuf[64];
>  	char			owner[OWNER_BUF_SZ];
>  	int			sunit, swidth;
>  	int			flg = 0;
> -- 
> 2.14.3
> 
> --
> 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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
  2017-12-05 23:56   ` Ross Zwisler
@ 2017-12-06  0:27     ` Dave Chinner
  -1 siblings, 0 replies; 42+ messages in thread
From: Dave Chinner @ 2017-12-06  0:27 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, Darrick J . Wong, linux-nvdimm, fstests, linux-xfs

On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> I recently upgraded my compiler from
> 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> to
> 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> and started getting a bunch of compiler warnings in io/fsmap.c:
> 
>   fsmap.c: In function ‘fsmap_f’:
>   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
>   between 1 and 17 bytes into a region of size between 12 and 28
>   [-Wformat-truncation=]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~
>   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~~~~~~~~~~~~
>   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
>   destination of size 32
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical),
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> signed long long can potentially print as many as 19 characters
> (9223372036854775807 is the max value).  The format we're using for bbuf is
> "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> "[..]:\0", which means it's possible we'll print up to 44 characters,
> overflowing our 32 char buffer.
> 
> Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> characters.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Darrick J. Wong <darrick.wong@oracle.com>
> Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")

FYI, I posted a fix for this weeks ago. I think Eric has already
picked it up, but it hasn't been pushed out into the for-next branch
yet.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
@ 2017-12-06  0:27     ` Dave Chinner
  0 siblings, 0 replies; 42+ messages in thread
From: Dave Chinner @ 2017-12-06  0:27 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dan Williams,
	Darrick J . Wong

On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> I recently upgraded my compiler from
> 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> to
> 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> and started getting a bunch of compiler warnings in io/fsmap.c:
> 
>   fsmap.c: In function ‘fsmap_f’:
>   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
>   between 1 and 17 bytes into a region of size between 12 and 28
>   [-Wformat-truncation=]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~
>   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>   ^~~~~~~~~~~~~~~
>   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
>   destination of size 32
>      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical),
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> signed long long can potentially print as many as 19 characters
> (9223372036854775807 is the max value).  The format we're using for bbuf is
> "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> "[..]:\0", which means it's possible we'll print up to 44 characters,
> overflowing our 32 char buffer.
> 
> Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> characters.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Darrick J. Wong <darrick.wong@oracle.com>
> Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")

FYI, I posted a fix for this weeks ago. I think Eric has already
picked it up, but it hasn't been pushed out into the for-next branch
yet.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
  2017-12-05 23:56   ` Ross Zwisler
@ 2017-12-06  0:29     ` Dave Chinner
  -1 siblings, 0 replies; 42+ messages in thread
From: Dave Chinner @ 2017-12-06  0:29 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: linux-xfs, Jan Kara, fstests, linux-nvdimm

On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> log marks.  It's helpful to allow users of xfs_io to adds these marks from
> within xfs_io instead of waiting until after xfs_io exits because then they
> are able to replay the dm-log-writes log up to immediately after another
> xfs_io operation such as mwrite.  This isolates the log replay from other
> operations that happen as part of xfs_io exiting (file handles being
> closed, mmaps being torn down, etc.).  This also allows users to insert
> multiple marks between different xfs_io commands.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Suggested-by: Dave Chinner <david@fromorbit.com>

having just looked at the xfs_io mess, this new command needs to set
"exitcode = 1" on failure. I haven't looked at anything else in the
patch.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
@ 2017-12-06  0:29     ` Dave Chinner
  0 siblings, 0 replies; 42+ messages in thread
From: Dave Chinner @ 2017-12-06  0:29 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dan Williams

On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> log marks.  It's helpful to allow users of xfs_io to adds these marks from
> within xfs_io instead of waiting until after xfs_io exits because then they
> are able to replay the dm-log-writes log up to immediately after another
> xfs_io operation such as mwrite.  This isolates the log replay from other
> operations that happen as part of xfs_io exiting (file handles being
> closed, mmaps being torn down, etc.).  This also allows users to insert
> multiple marks between different xfs_io commands.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Suggested-by: Dave Chinner <david@fromorbit.com>

having just looked at the xfs_io mess, this new command needs to set
"exitcode = 1" on failure. I haven't looked at anything else in the
patch.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
  2017-12-06  0:29     ` Dave Chinner
@ 2017-12-06  4:38       ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06  4:38 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Jan Kara, linux-nvdimm, fstests, linux-xfs

On Wed, Dec 06, 2017 at 11:29:46AM +1100, Dave Chinner wrote:
> On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> > log marks.  It's helpful to allow users of xfs_io to adds these marks from
> > within xfs_io instead of waiting until after xfs_io exits because then they
> > are able to replay the dm-log-writes log up to immediately after another
> > xfs_io operation such as mwrite.  This isolates the log replay from other
> > operations that happen as part of xfs_io exiting (file handles being
> > closed, mmaps being torn down, etc.).  This also allows users to insert
> > multiple marks between different xfs_io commands.
> > 
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Suggested-by: Dave Chinner <david@fromorbit.com>
> 
> having just looked at the xfs_io mess, this new command needs to set
> "exitcode = 1" on failure. I haven't looked at anything else in the
> patch.

Ah, yea, I did think it was odd that when hitting errors in xfs_io it always
still returned 0.  I'll fix up for the next version.  Is someone else working
on making the rest of the error cases in xfs_io return 1 as well, or should I
throw that at the front of the series?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
@ 2017-12-06  4:38       ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06  4:38 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Ross Zwisler, linux-xfs, linux-nvdimm, fstests, Jan Kara, Dan Williams

On Wed, Dec 06, 2017 at 11:29:46AM +1100, Dave Chinner wrote:
> On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> > log marks.  It's helpful to allow users of xfs_io to adds these marks from
> > within xfs_io instead of waiting until after xfs_io exits because then they
> > are able to replay the dm-log-writes log up to immediately after another
> > xfs_io operation such as mwrite.  This isolates the log replay from other
> > operations that happen as part of xfs_io exiting (file handles being
> > closed, mmaps being torn down, etc.).  This also allows users to insert
> > multiple marks between different xfs_io commands.
> > 
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Suggested-by: Dave Chinner <david@fromorbit.com>
> 
> having just looked at the xfs_io mess, this new command needs to set
> "exitcode = 1" on failure. I haven't looked at anything else in the
> patch.

Ah, yea, I did think it was odd that when hitting errors in xfs_io it always
still returned 0.  I'll fix up for the next version.  Is someone else working
on making the rest of the error cases in xfs_io return 1 as well, or should I
throw that at the front of the series?

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
  2017-12-06  4:38       ` Ross Zwisler
@ 2017-12-06  4:41         ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06  4:41 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Tue, Dec 05, 2017 at 09:38:13PM -0700, Ross Zwisler wrote:
> On Wed, Dec 06, 2017 at 11:29:46AM +1100, Dave Chinner wrote:
> > On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> > > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> > > log marks.  It's helpful to allow users of xfs_io to adds these marks from
> > > within xfs_io instead of waiting until after xfs_io exits because then they
> > > are able to replay the dm-log-writes log up to immediately after another
> > > xfs_io operation such as mwrite.  This isolates the log replay from other
> > > operations that happen as part of xfs_io exiting (file handles being
> > > closed, mmaps being torn down, etc.).  This also allows users to insert
> > > multiple marks between different xfs_io commands.
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > Suggested-by: Dave Chinner <david@fromorbit.com>
> > 
> > having just looked at the xfs_io mess, this new command needs to set
> > "exitcode = 1" on failure. I haven't looked at anything else in the
> > patch.
> 
> Ah, yea, I did think it was odd that when hitting errors in xfs_io it always
> still returned 0.  I'll fix up for the next version.  Is someone else working
> on making the rest of the error cases in xfs_io return 1 as well, or should I
> throw that at the front of the series?

Ah, nevermind, I found your patch that sets exitcode throughout xfs_io.  I'll
fix up for the next version, thanks for the review.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
@ 2017-12-06  4:41         ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06  4:41 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Dave Chinner, linux-xfs, linux-nvdimm, fstests, Jan Kara, Dan Williams

On Tue, Dec 05, 2017 at 09:38:13PM -0700, Ross Zwisler wrote:
> On Wed, Dec 06, 2017 at 11:29:46AM +1100, Dave Chinner wrote:
> > On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> > > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> > > log marks.  It's helpful to allow users of xfs_io to adds these marks from
> > > within xfs_io instead of waiting until after xfs_io exits because then they
> > > are able to replay the dm-log-writes log up to immediately after another
> > > xfs_io operation such as mwrite.  This isolates the log replay from other
> > > operations that happen as part of xfs_io exiting (file handles being
> > > closed, mmaps being torn down, etc.).  This also allows users to insert
> > > multiple marks between different xfs_io commands.
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > Suggested-by: Dave Chinner <david@fromorbit.com>
> > 
> > having just looked at the xfs_io mess, this new command needs to set
> > "exitcode = 1" on failure. I haven't looked at anything else in the
> > patch.
> 
> Ah, yea, I did think it was odd that when hitting errors in xfs_io it always
> still returned 0.  I'll fix up for the next version.  Is someone else working
> on making the rest of the error cases in xfs_io return 1 as well, or should I
> throw that at the front of the series?

Ah, nevermind, I found your patch that sets exitcode throughout xfs_io.  I'll
fix up for the next version, thanks for the review.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
  2017-12-06  4:38       ` Ross Zwisler
@ 2017-12-06  5:43         ` Dave Chinner
  -1 siblings, 0 replies; 42+ messages in thread
From: Dave Chinner @ 2017-12-06  5:43 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: linux-xfs, Jan Kara, fstests, linux-nvdimm

On Tue, Dec 05, 2017 at 09:38:13PM -0700, Ross Zwisler wrote:
> On Wed, Dec 06, 2017 at 11:29:46AM +1100, Dave Chinner wrote:
> > On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> > > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> > > log marks.  It's helpful to allow users of xfs_io to adds these marks from
> > > within xfs_io instead of waiting until after xfs_io exits because then they
> > > are able to replay the dm-log-writes log up to immediately after another
> > > xfs_io operation such as mwrite.  This isolates the log replay from other
> > > operations that happen as part of xfs_io exiting (file handles being
> > > closed, mmaps being torn down, etc.).  This also allows users to insert
> > > multiple marks between different xfs_io commands.
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > Suggested-by: Dave Chinner <david@fromorbit.com>
> > 
> > having just looked at the xfs_io mess, this new command needs to set
> > "exitcode = 1" on failure. I haven't looked at anything else in the
> > patch.
> 
> Ah, yea, I did think it was odd that when hitting errors in xfs_io it always
> still returned 0.  I'll fix up for the next version.  Is someone else working
> on making the rest of the error cases in xfs_io return 1 as well, or should I
> throw that at the front of the series?

I sent a patch earlier today to do exactly that.

https://marc.info/?l=fstests&m=151252000511167&w=2

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command
@ 2017-12-06  5:43         ` Dave Chinner
  0 siblings, 0 replies; 42+ messages in thread
From: Dave Chinner @ 2017-12-06  5:43 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dan Williams

On Tue, Dec 05, 2017 at 09:38:13PM -0700, Ross Zwisler wrote:
> On Wed, Dec 06, 2017 at 11:29:46AM +1100, Dave Chinner wrote:
> > On Tue, Dec 05, 2017 at 04:56:51PM -0700, Ross Zwisler wrote:
> > > Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> > > log marks.  It's helpful to allow users of xfs_io to adds these marks from
> > > within xfs_io instead of waiting until after xfs_io exits because then they
> > > are able to replay the dm-log-writes log up to immediately after another
> > > xfs_io operation such as mwrite.  This isolates the log replay from other
> > > operations that happen as part of xfs_io exiting (file handles being
> > > closed, mmaps being torn down, etc.).  This also allows users to insert
> > > multiple marks between different xfs_io commands.
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > Suggested-by: Dave Chinner <david@fromorbit.com>
> > 
> > having just looked at the xfs_io mess, this new command needs to set
> > "exitcode = 1" on failure. I haven't looked at anything else in the
> > patch.
> 
> Ah, yea, I did think it was odd that when hitting errors in xfs_io it always
> still returned 0.  I'll fix up for the next version.  Is someone else working
> on making the rest of the error cases in xfs_io return 1 as well, or should I
> throw that at the front of the series?

I sent a patch earlier today to do exactly that.

https://marc.info/?l=fstests&m=151252000511167&w=2

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
  2017-12-06  0:27     ` Dave Chinner
@ 2017-12-06 13:59       ` Eric Sandeen
  -1 siblings, 0 replies; 42+ messages in thread
From: Eric Sandeen @ 2017-12-06 13:59 UTC (permalink / raw)
  To: Dave Chinner, Ross Zwisler
  Cc: Jan Kara, Darrick J . Wong, linux-nvdimm, fstests, linux-xfs

On 12/5/17 6:27 PM, Dave Chinner wrote:

> FYI, I posted a fix for this weeks ago. I think Eric has already
> picked it up, but it hasn't been pushed out into the for-next branch
> yet.

Yes, sorry for the delay.  Will be pushed out soon.
 
> Cheers,
> 
> Dave.
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
@ 2017-12-06 13:59       ` Eric Sandeen
  0 siblings, 0 replies; 42+ messages in thread
From: Eric Sandeen @ 2017-12-06 13:59 UTC (permalink / raw)
  To: Dave Chinner, Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dan Williams,
	Darrick J . Wong

On 12/5/17 6:27 PM, Dave Chinner wrote:

> FYI, I posted a fix for this weeks ago. I think Eric has already
> picked it up, but it hasn't been pushed out into the for-next branch
> yet.

Yes, sorry for the delay.  Will be pushed out soon.
 
> Cheers,
> 
> Dave.
> 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v3 3/3] xfs_io: add a new 'log_writes' command
  2017-12-05 23:56   ` Ross Zwisler
@ 2017-12-06 18:13     ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06 18:13 UTC (permalink / raw)
  To: linux-xfs; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests

Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
log marks.  It's helpful to allow users of xfs_io to adds these marks from
within xfs_io instead of waiting until after xfs_io exits because then they
are able to replay the dm-log-writes log up to immediately after another
xfs_io operation such as mwrite.  This isolates the log replay from other
operations that happen as part of xfs_io exiting (file handles being
closed, mmaps being torn down, etc.).  This also allows users to insert
multiple marks between different xfs_io commands.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
---

Changes from v2:
 - Use 'exitcode' to have the xfs_io shell command error out with status
   1 if we hit any of our many error condtions during execution. (Dave)

---
 configure.ac            |   1 +
 debian/control          |   2 +-
 include/builddefs.in    |   2 +
 io/Makefile             |   6 +++
 io/init.c               |   1 +
 io/io.h                 |   6 +++
 io/log_writes.c         | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
 m4/Makefile             |   1 +
 m4/package_devmapper.m4 |  11 +++++
 man/man8/xfs_io.8       |  23 ++++++++++-
 10 files changed, 157 insertions(+), 2 deletions(-)
 create mode 100644 io/log_writes.c
 create mode 100644 m4/package_devmapper.m4

diff --git a/configure.ac b/configure.ac
index f3325aa0..f83d5817 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,6 +164,7 @@ AC_NEED_INTERNAL_FSXATTR
 AC_HAVE_GETFSMAP
 AC_HAVE_STATFS_FLAGS
 AC_HAVE_MAP_SYNC
+AC_HAVE_DEVMAPPER
 
 if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
diff --git a/debian/control b/debian/control
index ad816622..5c26e3d7 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
-Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev
+Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev
 Standards-Version: 3.9.1
 Homepage: http://xfs.org/
 
diff --git a/include/builddefs.in b/include/builddefs.in
index 126f7e95..66bdbfa2 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -35,6 +35,7 @@ LIBTERMCAP = @libtermcap@
 LIBEDITLINE = @libeditline@
 LIBREADLINE = @libreadline@
 LIBBLKID = @libblkid@
+LIBDEVMAPPER = @libdevmapper@
 LIBXFS = $(TOPDIR)/libxfs/libxfs.la
 LIBXCMD = $(TOPDIR)/libxcmd/libxcmd.la
 LIBXLOG = $(TOPDIR)/libxlog/libxlog.la
@@ -116,6 +117,7 @@ NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
 HAVE_GETFSMAP = @have_getfsmap@
 HAVE_STATFS_FLAGS = @have_statfs_flags@
 HAVE_MAP_SYNC = @have_map_sync@
+HAVE_DEVMAPPER = @have_devmapper@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
diff --git a/io/Makefile b/io/Makefile
index 2987ee11..b7585a1b 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -107,6 +107,12 @@ ifeq ($(HAVE_MAP_SYNC),yes)
 LCFLAGS += -DHAVE_MAP_SYNC
 endif
 
+ifeq ($(HAVE_DEVMAPPER),yes)
+CFILES += log_writes.c
+LLDLIBS += $(LIBDEVMAPPER)
+LCFLAGS += -DHAVE_DEVMAPPER
+endif
+
 # On linux we get fsmap from the system or define it ourselves
 # so include this based on platform type.  If this reverts to only
 # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
diff --git a/io/init.c b/io/init.c
index 20d5f80d..34d87b5f 100644
--- a/io/init.c
+++ b/io/init.c
@@ -72,6 +72,7 @@ init_commands(void)
 	help_init();
 	imap_init();
 	inject_init();
+	log_writes_init();
 	madvise_init();
 	mincore_init();
 	mmap_init();
diff --git a/io/io.h b/io/io.h
index 8b2753b3..9349cc75 100644
--- a/io/io.h
+++ b/io/io.h
@@ -186,3 +186,9 @@ extern void		fsmap_init(void);
 #else
 # define fsmap_init()	do { } while (0)
 #endif
+
+#ifdef HAVE_DEVMAPPER
+extern void		log_writes_init(void);
+#else
+#define log_writes_init()	do { } while (0)
+#endif
diff --git a/io/log_writes.c b/io/log_writes.c
new file mode 100644
index 00000000..c7b7392e
--- /dev/null
+++ b/io/log_writes.c
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "platform_defs.h"
+#include <libdevmapper.h>
+#include "command.h"
+#include "init.h"
+
+static cmdinfo_t log_writes_cmd;
+
+static int
+mark_log(const char *device, const char *mark)
+{
+	struct dm_task *dmt;
+	const int size = 80;
+	char message[size];
+	int len, ret = 0;
+
+	len = snprintf(message, size, "mark %s", mark);
+	if (len >= size) {
+		printf("mark '%s' is too long\n", mark);
+		return ret;
+	}
+
+	if (!(dmt = dm_task_create(DM_DEVICE_TARGET_MSG)))
+		return ret;
+
+	if (!dm_task_set_name(dmt, device))
+		goto out;
+
+	if (!dm_task_set_sector(dmt, 0))
+		goto out;
+
+	if (!dm_task_set_message(dmt, message))
+		goto out;
+
+	if (dm_task_run(dmt))
+		ret = 1;
+out:
+	dm_task_destroy(dmt);
+	return ret;
+}
+
+static int
+log_writes_f(
+	int			argc,
+	char			**argv)
+{
+	const char *device = NULL;
+	const char *mark = NULL;
+	int c;
+
+	exitcode = 1;
+	while ((c = getopt(argc, argv, "d:m:")) != EOF) {
+		switch (c) {
+		case 'd':
+			device = optarg;
+			break;
+		case 'm':
+			mark = optarg;
+			break;
+		default:
+			return command_usage(&log_writes_cmd);
+		}
+	}
+
+	if (device == NULL || mark == NULL)
+		return command_usage(&log_writes_cmd);
+
+	if (mark_log(device, mark))
+		exitcode = 0;
+
+	return 0;
+}
+
+void
+log_writes_init(void)
+{
+	log_writes_cmd.name = "log_writes";
+	log_writes_cmd.altname = "lw";
+	log_writes_cmd.cfunc = log_writes_f;
+	log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK
+				| CMD_FLAG_ONESHOT;
+	log_writes_cmd.argmin = 0;
+	log_writes_cmd.argmax = -1;
+	log_writes_cmd.args = _("-d device -m mark");
+	log_writes_cmd.oneline =
+		_("create mark <mark> in the dm-log-writes log specified by <device>");
+
+	add_command(&log_writes_cmd);
+}
diff --git a/m4/Makefile b/m4/Makefile
index 4706121b..77f2edda 100644
--- a/m4/Makefile
+++ b/m4/Makefile
@@ -15,6 +15,7 @@ CONFIGURE = \
 LSRCFILES = \
 	manual_format.m4 \
 	package_blkid.m4 \
+	package_devmapper.m4 \
 	package_globals.m4 \
 	package_libcdev.m4 \
 	package_pthread.m4 \
diff --git a/m4/package_devmapper.m4 b/m4/package_devmapper.m4
new file mode 100644
index 00000000..821f283c
--- /dev/null
+++ b/m4/package_devmapper.m4
@@ -0,0 +1,11 @@
+#
+# See if libdevmapper is available on the system.
+#
+AC_DEFUN([AC_HAVE_DEVMAPPER],
+[ AC_SEARCH_LIBS([dm_task_create], [devmapper],
+        libdevmapper="-ldevmapper"
+        have_devmapper=yes,
+        have_devmapper=no,)
+    AC_SUBST(have_devmapper)
+    AC_SUBST(libdevmapper)
+])
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 1693f7f1..4fce6d39 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1123,7 +1123,27 @@ version of policy structure (numeric)
 .BR get_encpolicy
 On filesystems that support encryption, display the encryption policy of the
 current file.
+.RE
+.PD
+.TP
+.BI "log_writes \-d " device " \-m "  mark
+Create a mark named
+.I mark
+in the dm-log-writes log specified by
+.I device.
+This is intended to be equivalent to the shell command:
 
+.B dmsetup message
+.I device
+.B 0 mark
+.I mark
+.PD
+.RE
+.TP
+.B lw
+See the
+.B log_writes
+command.
 .SH SEE ALSO
 .BR mkfs.xfs (8),
 .BR xfsctl (3),
@@ -1141,4 +1161,5 @@ current file.
 .BR open (2),
 .BR pread (2),
 .BR pwrite (2),
-.BR readdir (3).
+.BR readdir (3),
+.BR dmsetup (8).
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [xfsprogs PATCH v3 3/3] xfs_io: add a new 'log_writes' command
@ 2017-12-06 18:13     ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06 18:13 UTC (permalink / raw)
  To: linux-xfs
  Cc: Ross Zwisler, linux-nvdimm, fstests, Jan Kara, Dave Chinner,
	Dan Williams

Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
log marks.  It's helpful to allow users of xfs_io to adds these marks from
within xfs_io instead of waiting until after xfs_io exits because then they
are able to replay the dm-log-writes log up to immediately after another
xfs_io operation such as mwrite.  This isolates the log replay from other
operations that happen as part of xfs_io exiting (file handles being
closed, mmaps being torn down, etc.).  This also allows users to insert
multiple marks between different xfs_io commands.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
---

Changes from v2:
 - Use 'exitcode' to have the xfs_io shell command error out with status
   1 if we hit any of our many error condtions during execution. (Dave)

---
 configure.ac            |   1 +
 debian/control          |   2 +-
 include/builddefs.in    |   2 +
 io/Makefile             |   6 +++
 io/init.c               |   1 +
 io/io.h                 |   6 +++
 io/log_writes.c         | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
 m4/Makefile             |   1 +
 m4/package_devmapper.m4 |  11 +++++
 man/man8/xfs_io.8       |  23 ++++++++++-
 10 files changed, 157 insertions(+), 2 deletions(-)
 create mode 100644 io/log_writes.c
 create mode 100644 m4/package_devmapper.m4

diff --git a/configure.ac b/configure.ac
index f3325aa0..f83d5817 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,6 +164,7 @@ AC_NEED_INTERNAL_FSXATTR
 AC_HAVE_GETFSMAP
 AC_HAVE_STATFS_FLAGS
 AC_HAVE_MAP_SYNC
+AC_HAVE_DEVMAPPER
 
 if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
diff --git a/debian/control b/debian/control
index ad816622..5c26e3d7 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
-Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev
+Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev
 Standards-Version: 3.9.1
 Homepage: http://xfs.org/
 
diff --git a/include/builddefs.in b/include/builddefs.in
index 126f7e95..66bdbfa2 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -35,6 +35,7 @@ LIBTERMCAP = @libtermcap@
 LIBEDITLINE = @libeditline@
 LIBREADLINE = @libreadline@
 LIBBLKID = @libblkid@
+LIBDEVMAPPER = @libdevmapper@
 LIBXFS = $(TOPDIR)/libxfs/libxfs.la
 LIBXCMD = $(TOPDIR)/libxcmd/libxcmd.la
 LIBXLOG = $(TOPDIR)/libxlog/libxlog.la
@@ -116,6 +117,7 @@ NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
 HAVE_GETFSMAP = @have_getfsmap@
 HAVE_STATFS_FLAGS = @have_statfs_flags@
 HAVE_MAP_SYNC = @have_map_sync@
+HAVE_DEVMAPPER = @have_devmapper@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
diff --git a/io/Makefile b/io/Makefile
index 2987ee11..b7585a1b 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -107,6 +107,12 @@ ifeq ($(HAVE_MAP_SYNC),yes)
 LCFLAGS += -DHAVE_MAP_SYNC
 endif
 
+ifeq ($(HAVE_DEVMAPPER),yes)
+CFILES += log_writes.c
+LLDLIBS += $(LIBDEVMAPPER)
+LCFLAGS += -DHAVE_DEVMAPPER
+endif
+
 # On linux we get fsmap from the system or define it ourselves
 # so include this based on platform type.  If this reverts to only
 # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
diff --git a/io/init.c b/io/init.c
index 20d5f80d..34d87b5f 100644
--- a/io/init.c
+++ b/io/init.c
@@ -72,6 +72,7 @@ init_commands(void)
 	help_init();
 	imap_init();
 	inject_init();
+	log_writes_init();
 	madvise_init();
 	mincore_init();
 	mmap_init();
diff --git a/io/io.h b/io/io.h
index 8b2753b3..9349cc75 100644
--- a/io/io.h
+++ b/io/io.h
@@ -186,3 +186,9 @@ extern void		fsmap_init(void);
 #else
 # define fsmap_init()	do { } while (0)
 #endif
+
+#ifdef HAVE_DEVMAPPER
+extern void		log_writes_init(void);
+#else
+#define log_writes_init()	do { } while (0)
+#endif
diff --git a/io/log_writes.c b/io/log_writes.c
new file mode 100644
index 00000000..c7b7392e
--- /dev/null
+++ b/io/log_writes.c
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "platform_defs.h"
+#include <libdevmapper.h>
+#include "command.h"
+#include "init.h"
+
+static cmdinfo_t log_writes_cmd;
+
+static int
+mark_log(const char *device, const char *mark)
+{
+	struct dm_task *dmt;
+	const int size = 80;
+	char message[size];
+	int len, ret = 0;
+
+	len = snprintf(message, size, "mark %s", mark);
+	if (len >= size) {
+		printf("mark '%s' is too long\n", mark);
+		return ret;
+	}
+
+	if (!(dmt = dm_task_create(DM_DEVICE_TARGET_MSG)))
+		return ret;
+
+	if (!dm_task_set_name(dmt, device))
+		goto out;
+
+	if (!dm_task_set_sector(dmt, 0))
+		goto out;
+
+	if (!dm_task_set_message(dmt, message))
+		goto out;
+
+	if (dm_task_run(dmt))
+		ret = 1;
+out:
+	dm_task_destroy(dmt);
+	return ret;
+}
+
+static int
+log_writes_f(
+	int			argc,
+	char			**argv)
+{
+	const char *device = NULL;
+	const char *mark = NULL;
+	int c;
+
+	exitcode = 1;
+	while ((c = getopt(argc, argv, "d:m:")) != EOF) {
+		switch (c) {
+		case 'd':
+			device = optarg;
+			break;
+		case 'm':
+			mark = optarg;
+			break;
+		default:
+			return command_usage(&log_writes_cmd);
+		}
+	}
+
+	if (device == NULL || mark == NULL)
+		return command_usage(&log_writes_cmd);
+
+	if (mark_log(device, mark))
+		exitcode = 0;
+
+	return 0;
+}
+
+void
+log_writes_init(void)
+{
+	log_writes_cmd.name = "log_writes";
+	log_writes_cmd.altname = "lw";
+	log_writes_cmd.cfunc = log_writes_f;
+	log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK
+				| CMD_FLAG_ONESHOT;
+	log_writes_cmd.argmin = 0;
+	log_writes_cmd.argmax = -1;
+	log_writes_cmd.args = _("-d device -m mark");
+	log_writes_cmd.oneline =
+		_("create mark <mark> in the dm-log-writes log specified by <device>");
+
+	add_command(&log_writes_cmd);
+}
diff --git a/m4/Makefile b/m4/Makefile
index 4706121b..77f2edda 100644
--- a/m4/Makefile
+++ b/m4/Makefile
@@ -15,6 +15,7 @@ CONFIGURE = \
 LSRCFILES = \
 	manual_format.m4 \
 	package_blkid.m4 \
+	package_devmapper.m4 \
 	package_globals.m4 \
 	package_libcdev.m4 \
 	package_pthread.m4 \
diff --git a/m4/package_devmapper.m4 b/m4/package_devmapper.m4
new file mode 100644
index 00000000..821f283c
--- /dev/null
+++ b/m4/package_devmapper.m4
@@ -0,0 +1,11 @@
+#
+# See if libdevmapper is available on the system.
+#
+AC_DEFUN([AC_HAVE_DEVMAPPER],
+[ AC_SEARCH_LIBS([dm_task_create], [devmapper],
+        libdevmapper="-ldevmapper"
+        have_devmapper=yes,
+        have_devmapper=no,)
+    AC_SUBST(have_devmapper)
+    AC_SUBST(libdevmapper)
+])
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 1693f7f1..4fce6d39 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -1123,7 +1123,27 @@ version of policy structure (numeric)
 .BR get_encpolicy
 On filesystems that support encryption, display the encryption policy of the
 current file.
+.RE
+.PD
+.TP
+.BI "log_writes \-d " device " \-m "  mark
+Create a mark named
+.I mark
+in the dm-log-writes log specified by
+.I device.
+This is intended to be equivalent to the shell command:
 
+.B dmsetup message
+.I device
+.B 0 mark
+.I mark
+.PD
+.RE
+.TP
+.B lw
+See the
+.B log_writes
+command.
 .SH SEE ALSO
 .BR mkfs.xfs (8),
 .BR xfsctl (3),
@@ -1141,4 +1161,5 @@ current file.
 .BR open (2),
 .BR pread (2),
 .BR pwrite (2),
-.BR readdir (3).
+.BR readdir (3),
+.BR dmsetup (8).
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
  2017-12-06  0:27     ` Dave Chinner
@ 2017-12-06 20:10       ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06 20:10 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Jan Kara, linux-nvdimm, Darrick J . Wong, fstests, linux-xfs

On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote:
> On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> > I recently upgraded my compiler from
> > 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> > to
> > 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> > and started getting a bunch of compiler warnings in io/fsmap.c:
> > 
> >   fsmap.c: In function ‘fsmap_f’:
> >   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
> >   between 1 and 17 bytes into a region of size between 12 and 28
> >   [-Wformat-truncation=]
> >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> >   ^~~~
> >   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
> >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> >   ^~~~~~~~~~~~~~~
> >   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
> >   destination of size 32
> >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   (long long)BTOBBT(p->fmr_physical),
> >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
> >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> > signed long long can potentially print as many as 19 characters
> > (9223372036854775807 is the max value).  The format we're using for bbuf is
> > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> > "[..]:\0", which means it's possible we'll print up to 44 characters,
> > overflowing our 32 char buffer.
> > 
> > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> > characters.
> > 
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> 
> FYI, I posted a fix for this weeks ago. I think Eric has already
> picked it up, but it hasn't been pushed out into the for-next branch
> yet.

I'm seeing similar new compiler warnings when compiling xfstests:

write_log.c: In function ‘wlog_open’:
write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=]
    "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
                                     ^~
write_log.c:124:4: note: directive argument in the range [1089, 2047]
    "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

etc.

I don't see any patches posted that fix these, as of yet.  As far as you know,
am I correct in thinking that these still need to be fixed?

- Ross
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
@ 2017-12-06 20:10       ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06 20:10 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Ross Zwisler, linux-xfs, linux-nvdimm, fstests, Jan Kara,
	Dan Williams, Darrick J . Wong

On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote:
> On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> > I recently upgraded my compiler from
> > 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> > to
> > 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> > and started getting a bunch of compiler warnings in io/fsmap.c:
> > 
> >   fsmap.c: In function ‘fsmap_f’:
> >   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
> >   between 1 and 17 bytes into a region of size between 12 and 28
> >   [-Wformat-truncation=]
> >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> >   ^~~~
> >   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
> >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> >   ^~~~~~~~~~~~~~~
> >   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
> >   destination of size 32
> >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   (long long)BTOBBT(p->fmr_physical),
> >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
> >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> > signed long long can potentially print as many as 19 characters
> > (9223372036854775807 is the max value).  The format we're using for bbuf is
> > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> > "[..]:\0", which means it's possible we'll print up to 44 characters,
> > overflowing our 32 char buffer.
> > 
> > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> > characters.
> > 
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> 
> FYI, I posted a fix for this weeks ago. I think Eric has already
> picked it up, but it hasn't been pushed out into the for-next branch
> yet.

I'm seeing similar new compiler warnings when compiling xfstests:

write_log.c: In function ‘wlog_open’:
write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=]
    "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
                                     ^~
write_log.c:124:4: note: directive argument in the range [1089, 2047]
    "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

etc.

I don't see any patches posted that fix these, as of yet.  As far as you know,
am I correct in thinking that these still need to be fixed?

- Ross

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
  2017-12-06 20:10       ` Ross Zwisler
@ 2017-12-06 20:47         ` Darrick J. Wong
  -1 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-06 20:47 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Wed, Dec 06, 2017 at 01:10:14PM -0700, Ross Zwisler wrote:
> On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote:
> > On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> > > I recently upgraded my compiler from
> > > 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> > > to
> > > 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> > > and started getting a bunch of compiler warnings in io/fsmap.c:
> > > 
> > >   fsmap.c: In function ‘fsmap_f’:
> > >   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
> > >   between 1 and 17 bytes into a region of size between 12 and 28
> > >   [-Wformat-truncation=]
> > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > >   ^~~~
> > >   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
> > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > >   ^~~~~~~~~~~~~~~
> > >   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
> > >   destination of size 32
> > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >   (long long)BTOBBT(p->fmr_physical),
> > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
> > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> > > signed long long can potentially print as many as 19 characters
> > > (9223372036854775807 is the max value).  The format we're using for bbuf is
> > > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> > > "[..]:\0", which means it's possible we'll print up to 44 characters,
> > > overflowing our 32 char buffer.
> > > 
> > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> > > characters.
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> > 
> > FYI, I posted a fix for this weeks ago. I think Eric has already
> > picked it up, but it hasn't been pushed out into the for-next branch
> > yet.
> 
> I'm seeing similar new compiler warnings when compiling xfstests:
> 
> write_log.c: In function ‘wlog_open’:
> write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=]
>     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
>                                      ^~
> write_log.c:124:4: note: directive argument in the range [1089, 2047]
>     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> etc.
> 
> I don't see any patches posted that fix these, as of yet.  As far as you know,
> am I correct in thinking that these still need to be fixed?

It sure looks that way.

--D

> - Ross
> --
> 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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
@ 2017-12-06 20:47         ` Darrick J. Wong
  0 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-06 20:47 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Dave Chinner, linux-xfs, linux-nvdimm, fstests, Jan Kara, Dan Williams

On Wed, Dec 06, 2017 at 01:10:14PM -0700, Ross Zwisler wrote:
> On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote:
> > On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> > > I recently upgraded my compiler from
> > > 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> > > to
> > > 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> > > and started getting a bunch of compiler warnings in io/fsmap.c:
> > > 
> > >   fsmap.c: In function ‘fsmap_f’:
> > >   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
> > >   between 1 and 17 bytes into a region of size between 12 and 28
> > >   [-Wformat-truncation=]
> > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > >   ^~~~
> > >   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
> > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > >   ^~~~~~~~~~~~~~~
> > >   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
> > >   destination of size 32
> > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >   (long long)BTOBBT(p->fmr_physical),
> > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
> > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> > > signed long long can potentially print as many as 19 characters
> > > (9223372036854775807 is the max value).  The format we're using for bbuf is
> > > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> > > "[..]:\0", which means it's possible we'll print up to 44 characters,
> > > overflowing our 32 char buffer.
> > > 
> > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> > > characters.
> > > 
> > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> > 
> > FYI, I posted a fix for this weeks ago. I think Eric has already
> > picked it up, but it hasn't been pushed out into the for-next branch
> > yet.
> 
> I'm seeing similar new compiler warnings when compiling xfstests:
> 
> write_log.c: In function ‘wlog_open’:
> write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=]
>     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
>                                      ^~
> write_log.c:124:4: note: directive argument in the range [1089, 2047]
>     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> etc.
> 
> I don't see any patches posted that fix these, as of yet.  As far as you know,
> am I correct in thinking that these still need to be fixed?

It sure looks that way.

--D

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
  2017-12-06 20:47         ` Darrick J. Wong
@ 2017-12-06 20:58           ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06 20:58 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Wed, Dec 06, 2017 at 12:47:49PM -0800, Darrick J. Wong wrote:
> On Wed, Dec 06, 2017 at 01:10:14PM -0700, Ross Zwisler wrote:
> > On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote:
> > > On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> > > > I recently upgraded my compiler from
> > > > 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> > > > to
> > > > 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> > > > and started getting a bunch of compiler warnings in io/fsmap.c:
> > > > 
> > > >   fsmap.c: In function ‘fsmap_f’:
> > > >   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
> > > >   between 1 and 17 bytes into a region of size between 12 and 28
> > > >   [-Wformat-truncation=]
> > > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > > >   ^~~~
> > > >   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
> > > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > > >   ^~~~~~~~~~~~~~~
> > > >   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
> > > >   destination of size 32
> > > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > > >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >   (long long)BTOBBT(p->fmr_physical),
> > > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
> > > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > 
> > > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> > > > signed long long can potentially print as many as 19 characters
> > > > (9223372036854775807 is the max value).  The format we're using for bbuf is
> > > > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> > > > "[..]:\0", which means it's possible we'll print up to 44 characters,
> > > > overflowing our 32 char buffer.
> > > > 
> > > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> > > > characters.
> > > > 
> > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > > > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> > > 
> > > FYI, I posted a fix for this weeks ago. I think Eric has already
> > > picked it up, but it hasn't been pushed out into the for-next branch
> > > yet.
> > 
> > I'm seeing similar new compiler warnings when compiling xfstests:
> > 
> > write_log.c: In function ‘wlog_open’:
> > write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=]
> >     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
> >                                      ^~
> > write_log.c:124:4: note: directive argument in the range [1089, 2047]
> >     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
> >     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > etc.
> > 
> > I don't see any patches posted that fix these, as of yet.  As far as you know,
> > am I correct in thinking that these still need to be fixed?
> 
> It sure looks that way.

Cool, thanks.  I'll take a crack at them.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code
@ 2017-12-06 20:58           ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-06 20:58 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Ross Zwisler, Dave Chinner, linux-xfs, linux-nvdimm, fstests,
	Jan Kara, Dan Williams

On Wed, Dec 06, 2017 at 12:47:49PM -0800, Darrick J. Wong wrote:
> On Wed, Dec 06, 2017 at 01:10:14PM -0700, Ross Zwisler wrote:
> > On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote:
> > > On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote:
> > > > I recently upgraded my compiler from
> > > > 	gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
> > > > to
> > > > 	gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
> > > > and started getting a bunch of compiler warnings in io/fsmap.c:
> > > > 
> > > >   fsmap.c: In function ‘fsmap_f’:
> > > >   fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing
> > > >   between 1 and 17 bytes into a region of size between 12 and 28
> > > >   [-Wformat-truncation=]
> > > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > > >   ^~~~
> > > >   fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967]
> > > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > > >   ^~~~~~~~~~~~~~~
> > > >   fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a
> > > >   destination of size 32
> > > >      snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:",
> > > >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >   (long long)BTOBBT(p->fmr_physical),
> > > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >   (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1));
> > > >   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > 
> > > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each
> > > > signed long long can potentially print as many as 19 characters
> > > > (9223372036854775807 is the max value).  The format we're using for bbuf is
> > > > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters
> > > > "[..]:\0", which means it's possible we'll print up to 44 characters,
> > > > overflowing our 32 char buffer.
> > > > 
> > > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64
> > > > characters.
> > > > 
> > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > > > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl")
> > > 
> > > FYI, I posted a fix for this weeks ago. I think Eric has already
> > > picked it up, but it hasn't been pushed out into the for-next branch
> > > yet.
> > 
> > I'm seeing similar new compiler warnings when compiling xfstests:
> > 
> > write_log.c: In function ‘wlog_open’:
> > write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=]
> >     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
> >                                      ^~
> > write_log.c:124:4: note: directive argument in the range [1089, 2047]
> >     "Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
> >     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > etc.
> > 
> > I don't see any patches posted that fix these, as of yet.  As far as you know,
> > am I correct in thinking that these still need to be fixed?
> 
> It sure looks that way.

Cool, thanks.  I'll take a crack at them.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing
  2017-12-05 23:56 ` Ross Zwisler
@ 2017-12-13 16:45   ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-13 16:45 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Tue, Dec 05, 2017 at 04:56:48PM -0700, Ross Zwisler wrote:
> This is the second revision of my MAP_SYNC + dm-log-writes support for
> xfsprogs.

Friendly ping on this xfsprogs series.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing
@ 2017-12-13 16:45   ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-13 16:45 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dave Chinner, Dan Williams

On Tue, Dec 05, 2017 at 04:56:48PM -0700, Ross Zwisler wrote:
> This is the second revision of my MAP_SYNC + dm-log-writes support for
> xfsprogs.

Friendly ping on this xfsprogs series.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing
  2017-12-13 16:45   ` Ross Zwisler
@ 2017-12-21 16:55     ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-21 16:55 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Wed, Dec 13, 2017 at 09:45:52AM -0700, Ross Zwisler wrote:
> On Tue, Dec 05, 2017 at 04:56:48PM -0700, Ross Zwisler wrote:
> > This is the second revision of my MAP_SYNC + dm-log-writes support for
> > xfsprogs.
> 
> Friendly ping on this xfsprogs series.

ping
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing
@ 2017-12-21 16:55     ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-21 16:55 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dave Chinner, Dan Williams

On Wed, Dec 13, 2017 at 09:45:52AM -0700, Ross Zwisler wrote:
> On Tue, Dec 05, 2017 at 04:56:48PM -0700, Ross Zwisler wrote:
> > This is the second revision of my MAP_SYNC + dm-log-writes support for
> > xfsprogs.
> 
> Friendly ping on this xfsprogs series.

ping

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
  2017-12-05 23:56   ` Ross Zwisler
@ 2017-12-21 17:09     ` Darrick J. Wong
  -1 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-21 17:09 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Tue, Dec 05, 2017 at 04:56:50PM -0700, Ross Zwisler wrote:
> Add support for a new -S flag to xfs_io's mmap command.  This opens the
> mapping with the (MAP_SYNC | MAP_SHARED_VALIDATE) flags instead of the
> standard MAP_SHARED flag.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Suggested-by: Dave Chinner <david@fromorbit.com>
> ---
>  configure.ac          |  1 +
>  include/builddefs.in  |  1 +
>  include/linux.h       |  8 ++++++++
>  io/Makefile           |  4 ++++
>  io/io.h               |  1 +
>  io/mmap.c             | 23 ++++++++++++++++++-----
>  m4/package_libcdev.m4 | 16 ++++++++++++++++
>  man/man8/xfs_io.8     |  6 +++++-
>  8 files changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index e5d0309f..f3325aa0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -163,6 +163,7 @@ AC_HAVE_MREMAP
>  AC_NEED_INTERNAL_FSXATTR
>  AC_HAVE_GETFSMAP
>  AC_HAVE_STATFS_FLAGS
> +AC_HAVE_MAP_SYNC
>  
>  if test "$enable_blkid" = yes; then
>  AC_HAVE_BLKID_TOPO
> diff --git a/include/builddefs.in b/include/builddefs.in
> index fd274ddc..126f7e95 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -115,6 +115,7 @@ HAVE_MREMAP = @have_mremap@
>  NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
>  HAVE_GETFSMAP = @have_getfsmap@
>  HAVE_STATFS_FLAGS = @have_statfs_flags@
> +HAVE_MAP_SYNC = @have_map_sync@
>  
>  GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
>  #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
> diff --git a/include/linux.h b/include/linux.h
> index 6ce344c5..1998941a 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -327,4 +327,12 @@ fsmap_advance(
>  #define HAVE_GETFSMAP
>  #endif /* HAVE_GETFSMAP */
>  
> +#ifndef HAVE_MAP_SYNC
> +#define MAP_SYNC 0
> +#define MAP_SHARED_VALIDATE 0
> +#else
> +#include <asm-generic/mman.h>
> +#include <asm-generic/mman-common.h>
> +#endif /* HAVE_MAP_SYNC */
> +
>  #endif	/* __XFS_LINUX_H__ */
> diff --git a/io/Makefile b/io/Makefile
> index 6725936d..2987ee11 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -103,6 +103,10 @@ ifeq ($(HAVE_MREMAP),yes)
>  LCFLAGS += -DHAVE_MREMAP
>  endif
>  
> +ifeq ($(HAVE_MAP_SYNC),yes)
> +LCFLAGS += -DHAVE_MAP_SYNC
> +endif
> +
>  # On linux we get fsmap from the system or define it ourselves
>  # so include this based on platform type.  If this reverts to only
>  # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
> diff --git a/io/io.h b/io/io.h
> index 3862985f..8b2753b3 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -65,6 +65,7 @@ typedef struct mmap_region {
>  	size_t		length;		/* length of mapping */
>  	off64_t		offset;		/* start offset into backing file */
>  	int		prot;		/* protection mode of the mapping */
> +	bool		map_sync;	/* is this a MAP_SYNC mapping? */
>  	char		*name;		/* name of backing file */
>  } mmap_region_t;
>  
> diff --git a/io/mmap.c b/io/mmap.c
> index 7a8150e3..84319f48 100644
> --- a/io/mmap.c
> +++ b/io/mmap.c
> @@ -42,7 +42,7 @@ print_mapping(
>  	int		index,
>  	int		braces)
>  {
> -	unsigned char	buffer[8] = { 0 };
> +	char		buffer[8] = { 0 };
>  	int		i;
>  
>  	static struct {
> @@ -57,6 +57,10 @@ print_mapping(
>  
>  	for (i = 0, p = pflags; p->prot != PROT_NONE; i++, p++)
>  		buffer[i] = (map->prot & p->prot) ? p->mode : '-';
> +
> +	if (map->map_sync)
> +		sprintf(&buffer[i], " S");
> +
>  	printf("%c%03d%c 0x%lx - 0x%lx %s  %14s (%lld : %ld)\n",
>  		braces? '[' : ' ', index, braces? ']' : ' ',
>  		(unsigned long)map->addr,
> @@ -146,6 +150,7 @@ mmap_help(void)
>  " -r -- map with PROT_READ protection\n"
>  " -w -- map with PROT_WRITE protection\n"
>  " -x -- map with PROT_EXEC protection\n"
> +" -S -- map with MAP_SYNC and MAP_SHARED_VALIDATE flags\n"
>  " -s <size> -- first do mmap(size)/munmap(size), try to reserve some free space\n"
>  " If no protection mode is specified, all are used by default.\n"
>  "\n"));
> @@ -161,7 +166,7 @@ mmap_f(
>  	void		*address = NULL;
>  	char		*filename;
>  	size_t		blocksize, sectsize;
> -	int		c, prot = 0;
> +	int		c, prot = 0, flags = MAP_SHARED;
>  
>  	if (argc == 1) {
>  		if (mapping)
> @@ -184,7 +189,7 @@ mmap_f(
>  
>  	init_cvtnum(&blocksize, &sectsize);
>  
> -	while ((c = getopt(argc, argv, "rwxs:")) != EOF) {
> +	while ((c = getopt(argc, argv, "rwxSs:")) != EOF) {
>  		switch (c) {
>  		case 'r':
>  			prot |= PROT_READ;
> @@ -195,6 +200,13 @@ mmap_f(
>  		case 'x':
>  			prot |= PROT_EXEC;
>  			break;
> +		case 'S':
> +			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> +			if (!flags) {

Heh, subtle. :)

/me wonders if it'd be better to do this explicitly:

#ifdef HAVE_MAP_SYNC
	flags = MAP_SYNC | MAP_SHARED_VALIDATE;
#else
	printf("MAP_SYNC not supported\n");
	return 1;
#endif

...though it's ugly.

> +				printf("MAP_SYNC not supported\n");
> +				return 0;

Are we supposed to be returning nonzero values for failing commands?

> +			}
> +			break;
>  		case 's':
>  			length2 = cvtnum(blocksize, sectsize, optarg);
>  			break;
> @@ -238,7 +250,7 @@ mmap_f(
>  		               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>  		munmap(address, length2);
>  	}
> -	address = mmap(address, length, prot, MAP_SHARED, file->fd, offset);
> +	address = mmap(address, length, prot, flags, file->fd, offset);
>  	if (address == MAP_FAILED) {
>  		perror("mmap");
>  		free(filename);
> @@ -263,6 +275,7 @@ mmap_f(
>  	mapping->offset = offset;
>  	mapping->name = filename;
>  	mapping->prot = prot;
> +	mapping->map_sync = (flags == (MAP_SYNC | MAP_SHARED_VALIDATE));
>  	return 0;
>  }
>  
> @@ -676,7 +689,7 @@ mmap_init(void)
>  	mmap_cmd.argmax = -1;
>  	mmap_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
>  			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
> -	mmap_cmd.args = _("[N] | [-rwx] [-s size] [off len]");
> +	mmap_cmd.args = _("[N] | [-rwxS] [-s size] [off len]");
>  	mmap_cmd.oneline =
>  		_("mmap a range in the current file, show mappings");
>  	mmap_cmd.help = mmap_help;
> diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
> index 7b4dfc85..71cedc5c 100644
> --- a/m4/package_libcdev.m4
> +++ b/m4/package_libcdev.m4
> @@ -328,3 +328,19 @@ AC_DEFUN([AC_HAVE_STATFS_FLAGS],
>      )
>      AC_SUBST(have_statfs_flags)
>    ])
> +
> +#
> +# Check if we have MAP_SYNC defines (Linux)
> +#
> +AC_DEFUN([AC_HAVE_MAP_SYNC],
> +  [ AC_MSG_CHECKING([for MAP_SYNC])
> +    AC_TRY_COMPILE([
> +#include <asm-generic/mman.h>
> +#include <asm-generic/mman-common.h>
> +    ], [
> +        int flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> +    ], have_map_sync=yes
> +	AC_MSG_RESULT(yes),
> +	AC_MSG_RESULT(no))
> +    AC_SUBST(have_map_sync)
> +  ])
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 9bf1a478..1693f7f1 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
>  
>  .SH MEMORY MAPPED I/O COMMANDS
>  .TP
> -.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
> +.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
>  With no arguments,
>  .B mmap
>  shows the current mappings. Specifying a single numeric argument
> @@ -780,6 +780,10 @@ PROT_WRITE
>  .RB ( \-w ),
>  and PROT_EXEC
>  .RB ( \-x ).
> +The mapping will be created with the MAP_SHARED flag by default, or with the
> +Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if

I assume I'll be able to look up exactly what MAP_SYNC provides in the mmap
manpage, right?

--D

> +.B -S
> +is given.
>  .BI \-s " size"
>  is used to do a mmap(size) && munmap(size) operation at first, try to reserve some
>  extendible free memory space, if
> -- 
> 2.14.3
> 
> --
> 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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
@ 2017-12-21 17:09     ` Darrick J. Wong
  0 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-21 17:09 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dave Chinner, Dan Williams

On Tue, Dec 05, 2017 at 04:56:50PM -0700, Ross Zwisler wrote:
> Add support for a new -S flag to xfs_io's mmap command.  This opens the
> mapping with the (MAP_SYNC | MAP_SHARED_VALIDATE) flags instead of the
> standard MAP_SHARED flag.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Suggested-by: Dave Chinner <david@fromorbit.com>
> ---
>  configure.ac          |  1 +
>  include/builddefs.in  |  1 +
>  include/linux.h       |  8 ++++++++
>  io/Makefile           |  4 ++++
>  io/io.h               |  1 +
>  io/mmap.c             | 23 ++++++++++++++++++-----
>  m4/package_libcdev.m4 | 16 ++++++++++++++++
>  man/man8/xfs_io.8     |  6 +++++-
>  8 files changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index e5d0309f..f3325aa0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -163,6 +163,7 @@ AC_HAVE_MREMAP
>  AC_NEED_INTERNAL_FSXATTR
>  AC_HAVE_GETFSMAP
>  AC_HAVE_STATFS_FLAGS
> +AC_HAVE_MAP_SYNC
>  
>  if test "$enable_blkid" = yes; then
>  AC_HAVE_BLKID_TOPO
> diff --git a/include/builddefs.in b/include/builddefs.in
> index fd274ddc..126f7e95 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -115,6 +115,7 @@ HAVE_MREMAP = @have_mremap@
>  NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
>  HAVE_GETFSMAP = @have_getfsmap@
>  HAVE_STATFS_FLAGS = @have_statfs_flags@
> +HAVE_MAP_SYNC = @have_map_sync@
>  
>  GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
>  #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
> diff --git a/include/linux.h b/include/linux.h
> index 6ce344c5..1998941a 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -327,4 +327,12 @@ fsmap_advance(
>  #define HAVE_GETFSMAP
>  #endif /* HAVE_GETFSMAP */
>  
> +#ifndef HAVE_MAP_SYNC
> +#define MAP_SYNC 0
> +#define MAP_SHARED_VALIDATE 0
> +#else
> +#include <asm-generic/mman.h>
> +#include <asm-generic/mman-common.h>
> +#endif /* HAVE_MAP_SYNC */
> +
>  #endif	/* __XFS_LINUX_H__ */
> diff --git a/io/Makefile b/io/Makefile
> index 6725936d..2987ee11 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -103,6 +103,10 @@ ifeq ($(HAVE_MREMAP),yes)
>  LCFLAGS += -DHAVE_MREMAP
>  endif
>  
> +ifeq ($(HAVE_MAP_SYNC),yes)
> +LCFLAGS += -DHAVE_MAP_SYNC
> +endif
> +
>  # On linux we get fsmap from the system or define it ourselves
>  # so include this based on platform type.  If this reverts to only
>  # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
> diff --git a/io/io.h b/io/io.h
> index 3862985f..8b2753b3 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -65,6 +65,7 @@ typedef struct mmap_region {
>  	size_t		length;		/* length of mapping */
>  	off64_t		offset;		/* start offset into backing file */
>  	int		prot;		/* protection mode of the mapping */
> +	bool		map_sync;	/* is this a MAP_SYNC mapping? */
>  	char		*name;		/* name of backing file */
>  } mmap_region_t;
>  
> diff --git a/io/mmap.c b/io/mmap.c
> index 7a8150e3..84319f48 100644
> --- a/io/mmap.c
> +++ b/io/mmap.c
> @@ -42,7 +42,7 @@ print_mapping(
>  	int		index,
>  	int		braces)
>  {
> -	unsigned char	buffer[8] = { 0 };
> +	char		buffer[8] = { 0 };
>  	int		i;
>  
>  	static struct {
> @@ -57,6 +57,10 @@ print_mapping(
>  
>  	for (i = 0, p = pflags; p->prot != PROT_NONE; i++, p++)
>  		buffer[i] = (map->prot & p->prot) ? p->mode : '-';
> +
> +	if (map->map_sync)
> +		sprintf(&buffer[i], " S");
> +
>  	printf("%c%03d%c 0x%lx - 0x%lx %s  %14s (%lld : %ld)\n",
>  		braces? '[' : ' ', index, braces? ']' : ' ',
>  		(unsigned long)map->addr,
> @@ -146,6 +150,7 @@ mmap_help(void)
>  " -r -- map with PROT_READ protection\n"
>  " -w -- map with PROT_WRITE protection\n"
>  " -x -- map with PROT_EXEC protection\n"
> +" -S -- map with MAP_SYNC and MAP_SHARED_VALIDATE flags\n"
>  " -s <size> -- first do mmap(size)/munmap(size), try to reserve some free space\n"
>  " If no protection mode is specified, all are used by default.\n"
>  "\n"));
> @@ -161,7 +166,7 @@ mmap_f(
>  	void		*address = NULL;
>  	char		*filename;
>  	size_t		blocksize, sectsize;
> -	int		c, prot = 0;
> +	int		c, prot = 0, flags = MAP_SHARED;
>  
>  	if (argc == 1) {
>  		if (mapping)
> @@ -184,7 +189,7 @@ mmap_f(
>  
>  	init_cvtnum(&blocksize, &sectsize);
>  
> -	while ((c = getopt(argc, argv, "rwxs:")) != EOF) {
> +	while ((c = getopt(argc, argv, "rwxSs:")) != EOF) {
>  		switch (c) {
>  		case 'r':
>  			prot |= PROT_READ;
> @@ -195,6 +200,13 @@ mmap_f(
>  		case 'x':
>  			prot |= PROT_EXEC;
>  			break;
> +		case 'S':
> +			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> +			if (!flags) {

Heh, subtle. :)

/me wonders if it'd be better to do this explicitly:

#ifdef HAVE_MAP_SYNC
	flags = MAP_SYNC | MAP_SHARED_VALIDATE;
#else
	printf("MAP_SYNC not supported\n");
	return 1;
#endif

...though it's ugly.

> +				printf("MAP_SYNC not supported\n");
> +				return 0;

Are we supposed to be returning nonzero values for failing commands?

> +			}
> +			break;
>  		case 's':
>  			length2 = cvtnum(blocksize, sectsize, optarg);
>  			break;
> @@ -238,7 +250,7 @@ mmap_f(
>  		               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>  		munmap(address, length2);
>  	}
> -	address = mmap(address, length, prot, MAP_SHARED, file->fd, offset);
> +	address = mmap(address, length, prot, flags, file->fd, offset);
>  	if (address == MAP_FAILED) {
>  		perror("mmap");
>  		free(filename);
> @@ -263,6 +275,7 @@ mmap_f(
>  	mapping->offset = offset;
>  	mapping->name = filename;
>  	mapping->prot = prot;
> +	mapping->map_sync = (flags == (MAP_SYNC | MAP_SHARED_VALIDATE));
>  	return 0;
>  }
>  
> @@ -676,7 +689,7 @@ mmap_init(void)
>  	mmap_cmd.argmax = -1;
>  	mmap_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
>  			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
> -	mmap_cmd.args = _("[N] | [-rwx] [-s size] [off len]");
> +	mmap_cmd.args = _("[N] | [-rwxS] [-s size] [off len]");
>  	mmap_cmd.oneline =
>  		_("mmap a range in the current file, show mappings");
>  	mmap_cmd.help = mmap_help;
> diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
> index 7b4dfc85..71cedc5c 100644
> --- a/m4/package_libcdev.m4
> +++ b/m4/package_libcdev.m4
> @@ -328,3 +328,19 @@ AC_DEFUN([AC_HAVE_STATFS_FLAGS],
>      )
>      AC_SUBST(have_statfs_flags)
>    ])
> +
> +#
> +# Check if we have MAP_SYNC defines (Linux)
> +#
> +AC_DEFUN([AC_HAVE_MAP_SYNC],
> +  [ AC_MSG_CHECKING([for MAP_SYNC])
> +    AC_TRY_COMPILE([
> +#include <asm-generic/mman.h>
> +#include <asm-generic/mman-common.h>
> +    ], [
> +        int flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> +    ], have_map_sync=yes
> +	AC_MSG_RESULT(yes),
> +	AC_MSG_RESULT(no))
> +    AC_SUBST(have_map_sync)
> +  ])
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 9bf1a478..1693f7f1 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
>  
>  .SH MEMORY MAPPED I/O COMMANDS
>  .TP
> -.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
> +.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
>  With no arguments,
>  .B mmap
>  shows the current mappings. Specifying a single numeric argument
> @@ -780,6 +780,10 @@ PROT_WRITE
>  .RB ( \-w ),
>  and PROT_EXEC
>  .RB ( \-x ).
> +The mapping will be created with the MAP_SHARED flag by default, or with the
> +Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if

I assume I'll be able to look up exactly what MAP_SYNC provides in the mmap
manpage, right?

--D

> +.B -S
> +is given.
>  .BI \-s " size"
>  is used to do a mmap(size) && munmap(size) operation at first, try to reserve some
>  extendible free memory space, if
> -- 
> 2.14.3
> 
> --
> 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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v3 3/3] xfs_io: add a new 'log_writes' command
  2017-12-06 18:13     ` Ross Zwisler
@ 2017-12-21 17:14       ` Darrick J. Wong
  -1 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-21 17:14 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Wed, Dec 06, 2017 at 11:13:23AM -0700, Ross Zwisler wrote:
> Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> log marks.  It's helpful to allow users of xfs_io to adds these marks from
> within xfs_io instead of waiting until after xfs_io exits because then they
> are able to replay the dm-log-writes log up to immediately after another
> xfs_io operation such as mwrite.  This isolates the log replay from other
> operations that happen as part of xfs_io exiting (file handles being
> closed, mmaps being torn down, etc.).  This also allows users to insert
> multiple marks between different xfs_io commands.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Suggested-by: Dave Chinner <david@fromorbit.com>
> ---
> 
> Changes from v2:
>  - Use 'exitcode' to have the xfs_io shell command error out with status
>    1 if we hit any of our many error condtions during execution. (Dave)
> 
> ---
>  configure.ac            |   1 +
>  debian/control          |   2 +-
>  include/builddefs.in    |   2 +
>  io/Makefile             |   6 +++
>  io/init.c               |   1 +
>  io/io.h                 |   6 +++
>  io/log_writes.c         | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
>  m4/Makefile             |   1 +
>  m4/package_devmapper.m4 |  11 +++++
>  man/man8/xfs_io.8       |  23 ++++++++++-
>  10 files changed, 157 insertions(+), 2 deletions(-)
>  create mode 100644 io/log_writes.c
>  create mode 100644 m4/package_devmapper.m4
> 
> diff --git a/configure.ac b/configure.ac
> index f3325aa0..f83d5817 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -164,6 +164,7 @@ AC_NEED_INTERNAL_FSXATTR
>  AC_HAVE_GETFSMAP
>  AC_HAVE_STATFS_FLAGS
>  AC_HAVE_MAP_SYNC
> +AC_HAVE_DEVMAPPER
>  
>  if test "$enable_blkid" = yes; then
>  AC_HAVE_BLKID_TOPO
> diff --git a/debian/control b/debian/control
> index ad816622..5c26e3d7 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -3,7 +3,7 @@ Section: admin
>  Priority: optional
>  Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
>  Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
> -Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev
> +Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev
>  Standards-Version: 3.9.1
>  Homepage: http://xfs.org/
>  
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 126f7e95..66bdbfa2 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -35,6 +35,7 @@ LIBTERMCAP = @libtermcap@
>  LIBEDITLINE = @libeditline@
>  LIBREADLINE = @libreadline@
>  LIBBLKID = @libblkid@
> +LIBDEVMAPPER = @libdevmapper@
>  LIBXFS = $(TOPDIR)/libxfs/libxfs.la
>  LIBXCMD = $(TOPDIR)/libxcmd/libxcmd.la
>  LIBXLOG = $(TOPDIR)/libxlog/libxlog.la
> @@ -116,6 +117,7 @@ NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
>  HAVE_GETFSMAP = @have_getfsmap@
>  HAVE_STATFS_FLAGS = @have_statfs_flags@
>  HAVE_MAP_SYNC = @have_map_sync@
> +HAVE_DEVMAPPER = @have_devmapper@
>  
>  GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
>  #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
> diff --git a/io/Makefile b/io/Makefile
> index 2987ee11..b7585a1b 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -107,6 +107,12 @@ ifeq ($(HAVE_MAP_SYNC),yes)
>  LCFLAGS += -DHAVE_MAP_SYNC
>  endif
>  
> +ifeq ($(HAVE_DEVMAPPER),yes)
> +CFILES += log_writes.c
> +LLDLIBS += $(LIBDEVMAPPER)
> +LCFLAGS += -DHAVE_DEVMAPPER
> +endif
> +
>  # On linux we get fsmap from the system or define it ourselves
>  # so include this based on platform type.  If this reverts to only
>  # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
> diff --git a/io/init.c b/io/init.c
> index 20d5f80d..34d87b5f 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -72,6 +72,7 @@ init_commands(void)
>  	help_init();
>  	imap_init();
>  	inject_init();
> +	log_writes_init();
>  	madvise_init();
>  	mincore_init();
>  	mmap_init();
> diff --git a/io/io.h b/io/io.h
> index 8b2753b3..9349cc75 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -186,3 +186,9 @@ extern void		fsmap_init(void);
>  #else
>  # define fsmap_init()	do { } while (0)
>  #endif
> +
> +#ifdef HAVE_DEVMAPPER
> +extern void		log_writes_init(void);
> +#else
> +#define log_writes_init()	do { } while (0)
> +#endif
> diff --git a/io/log_writes.c b/io/log_writes.c
> new file mode 100644
> index 00000000..c7b7392e
> --- /dev/null
> +++ b/io/log_writes.c
> @@ -0,0 +1,106 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation.
> + * All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#include "platform_defs.h"
> +#include <libdevmapper.h>
> +#include "command.h"
> +#include "init.h"
> +
> +static cmdinfo_t log_writes_cmd;
> +
> +static int
> +mark_log(const char *device, const char *mark)
> +{
> +	struct dm_task *dmt;
> +	const int size = 80;
> +	char message[size];
> +	int len, ret = 0;

Indentation?

> +
> +	len = snprintf(message, size, "mark %s", mark);
> +	if (len >= size) {
> +		printf("mark '%s' is too long\n", mark);
> +		return ret;
> +	}
> +
> +	if (!(dmt = dm_task_create(DM_DEVICE_TARGET_MSG)))
> +		return ret;
> +
> +	if (!dm_task_set_name(dmt, device))
> +		goto out;
> +
> +	if (!dm_task_set_sector(dmt, 0))
> +		goto out;
> +
> +	if (!dm_task_set_message(dmt, message))
> +		goto out;
> +
> +	if (dm_task_run(dmt))
> +		ret = 1;
> +out:
> +	dm_task_destroy(dmt);
> +	return ret;
> +}
> +
> +static int
> +log_writes_f(
> +	int			argc,
> +	char			**argv)
> +{
> +	const char *device = NULL;
> +	const char *mark = NULL;

	const char		*device = NULL;
	const char		*mark = NULL;

Yeah, variable declaration indentation in the functions was the only
thing I found to complain about...

Otherwise looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> +	int c;
> +
> +	exitcode = 1;
> +	while ((c = getopt(argc, argv, "d:m:")) != EOF) {
> +		switch (c) {
> +		case 'd':
> +			device = optarg;
> +			break;
> +		case 'm':
> +			mark = optarg;
> +			break;
> +		default:
> +			return command_usage(&log_writes_cmd);
> +		}
> +	}
> +
> +	if (device == NULL || mark == NULL)
> +		return command_usage(&log_writes_cmd);
> +
> +	if (mark_log(device, mark))
> +		exitcode = 0;
> +
> +	return 0;
> +}
> +
> +void
> +log_writes_init(void)
> +{
> +	log_writes_cmd.name = "log_writes";
> +	log_writes_cmd.altname = "lw";
> +	log_writes_cmd.cfunc = log_writes_f;
> +	log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK
> +				| CMD_FLAG_ONESHOT;
> +	log_writes_cmd.argmin = 0;
> +	log_writes_cmd.argmax = -1;
> +	log_writes_cmd.args = _("-d device -m mark");
> +	log_writes_cmd.oneline =
> +		_("create mark <mark> in the dm-log-writes log specified by <device>");
> +
> +	add_command(&log_writes_cmd);
> +}
> diff --git a/m4/Makefile b/m4/Makefile
> index 4706121b..77f2edda 100644
> --- a/m4/Makefile
> +++ b/m4/Makefile
> @@ -15,6 +15,7 @@ CONFIGURE = \
>  LSRCFILES = \
>  	manual_format.m4 \
>  	package_blkid.m4 \
> +	package_devmapper.m4 \
>  	package_globals.m4 \
>  	package_libcdev.m4 \
>  	package_pthread.m4 \
> diff --git a/m4/package_devmapper.m4 b/m4/package_devmapper.m4
> new file mode 100644
> index 00000000..821f283c
> --- /dev/null
> +++ b/m4/package_devmapper.m4
> @@ -0,0 +1,11 @@
> +#
> +# See if libdevmapper is available on the system.
> +#
> +AC_DEFUN([AC_HAVE_DEVMAPPER],
> +[ AC_SEARCH_LIBS([dm_task_create], [devmapper],
> +        libdevmapper="-ldevmapper"
> +        have_devmapper=yes,
> +        have_devmapper=no,)
> +    AC_SUBST(have_devmapper)
> +    AC_SUBST(libdevmapper)
> +])
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 1693f7f1..4fce6d39 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -1123,7 +1123,27 @@ version of policy structure (numeric)
>  .BR get_encpolicy
>  On filesystems that support encryption, display the encryption policy of the
>  current file.
> +.RE
> +.PD
> +.TP
> +.BI "log_writes \-d " device " \-m "  mark
> +Create a mark named
> +.I mark
> +in the dm-log-writes log specified by
> +.I device.
> +This is intended to be equivalent to the shell command:
>  
> +.B dmsetup message
> +.I device
> +.B 0 mark
> +.I mark
> +.PD
> +.RE
> +.TP
> +.B lw
> +See the
> +.B log_writes
> +command.
>  .SH SEE ALSO
>  .BR mkfs.xfs (8),
>  .BR xfsctl (3),
> @@ -1141,4 +1161,5 @@ current file.
>  .BR open (2),
>  .BR pread (2),
>  .BR pwrite (2),
> -.BR readdir (3).
> +.BR readdir (3),
> +.BR dmsetup (8).
> -- 
> 2.14.3
> 
> --
> 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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v3 3/3] xfs_io: add a new 'log_writes' command
@ 2017-12-21 17:14       ` Darrick J. Wong
  0 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-21 17:14 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dave Chinner, Dan Williams

On Wed, Dec 06, 2017 at 11:13:23AM -0700, Ross Zwisler wrote:
> Add a new 'log_writes' command to xfs_io so that we can add dm-log-writes
> log marks.  It's helpful to allow users of xfs_io to adds these marks from
> within xfs_io instead of waiting until after xfs_io exits because then they
> are able to replay the dm-log-writes log up to immediately after another
> xfs_io operation such as mwrite.  This isolates the log replay from other
> operations that happen as part of xfs_io exiting (file handles being
> closed, mmaps being torn down, etc.).  This also allows users to insert
> multiple marks between different xfs_io commands.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Suggested-by: Dave Chinner <david@fromorbit.com>
> ---
> 
> Changes from v2:
>  - Use 'exitcode' to have the xfs_io shell command error out with status
>    1 if we hit any of our many error condtions during execution. (Dave)
> 
> ---
>  configure.ac            |   1 +
>  debian/control          |   2 +-
>  include/builddefs.in    |   2 +
>  io/Makefile             |   6 +++
>  io/init.c               |   1 +
>  io/io.h                 |   6 +++
>  io/log_writes.c         | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
>  m4/Makefile             |   1 +
>  m4/package_devmapper.m4 |  11 +++++
>  man/man8/xfs_io.8       |  23 ++++++++++-
>  10 files changed, 157 insertions(+), 2 deletions(-)
>  create mode 100644 io/log_writes.c
>  create mode 100644 m4/package_devmapper.m4
> 
> diff --git a/configure.ac b/configure.ac
> index f3325aa0..f83d5817 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -164,6 +164,7 @@ AC_NEED_INTERNAL_FSXATTR
>  AC_HAVE_GETFSMAP
>  AC_HAVE_STATFS_FLAGS
>  AC_HAVE_MAP_SYNC
> +AC_HAVE_DEVMAPPER
>  
>  if test "$enable_blkid" = yes; then
>  AC_HAVE_BLKID_TOPO
> diff --git a/debian/control b/debian/control
> index ad816622..5c26e3d7 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -3,7 +3,7 @@ Section: admin
>  Priority: optional
>  Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
>  Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
> -Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev
> +Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev
>  Standards-Version: 3.9.1
>  Homepage: http://xfs.org/
>  
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 126f7e95..66bdbfa2 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -35,6 +35,7 @@ LIBTERMCAP = @libtermcap@
>  LIBEDITLINE = @libeditline@
>  LIBREADLINE = @libreadline@
>  LIBBLKID = @libblkid@
> +LIBDEVMAPPER = @libdevmapper@
>  LIBXFS = $(TOPDIR)/libxfs/libxfs.la
>  LIBXCMD = $(TOPDIR)/libxcmd/libxcmd.la
>  LIBXLOG = $(TOPDIR)/libxlog/libxlog.la
> @@ -116,6 +117,7 @@ NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@
>  HAVE_GETFSMAP = @have_getfsmap@
>  HAVE_STATFS_FLAGS = @have_statfs_flags@
>  HAVE_MAP_SYNC = @have_map_sync@
> +HAVE_DEVMAPPER = @have_devmapper@
>  
>  GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
>  #	   -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
> diff --git a/io/Makefile b/io/Makefile
> index 2987ee11..b7585a1b 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -107,6 +107,12 @@ ifeq ($(HAVE_MAP_SYNC),yes)
>  LCFLAGS += -DHAVE_MAP_SYNC
>  endif
>  
> +ifeq ($(HAVE_DEVMAPPER),yes)
> +CFILES += log_writes.c
> +LLDLIBS += $(LIBDEVMAPPER)
> +LCFLAGS += -DHAVE_DEVMAPPER
> +endif
> +
>  # On linux we get fsmap from the system or define it ourselves
>  # so include this based on platform type.  If this reverts to only
>  # the autoconf check w/o local definition, change to testing HAVE_GETFSMAP
> diff --git a/io/init.c b/io/init.c
> index 20d5f80d..34d87b5f 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -72,6 +72,7 @@ init_commands(void)
>  	help_init();
>  	imap_init();
>  	inject_init();
> +	log_writes_init();
>  	madvise_init();
>  	mincore_init();
>  	mmap_init();
> diff --git a/io/io.h b/io/io.h
> index 8b2753b3..9349cc75 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -186,3 +186,9 @@ extern void		fsmap_init(void);
>  #else
>  # define fsmap_init()	do { } while (0)
>  #endif
> +
> +#ifdef HAVE_DEVMAPPER
> +extern void		log_writes_init(void);
> +#else
> +#define log_writes_init()	do { } while (0)
> +#endif
> diff --git a/io/log_writes.c b/io/log_writes.c
> new file mode 100644
> index 00000000..c7b7392e
> --- /dev/null
> +++ b/io/log_writes.c
> @@ -0,0 +1,106 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation.
> + * All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#include "platform_defs.h"
> +#include <libdevmapper.h>
> +#include "command.h"
> +#include "init.h"
> +
> +static cmdinfo_t log_writes_cmd;
> +
> +static int
> +mark_log(const char *device, const char *mark)
> +{
> +	struct dm_task *dmt;
> +	const int size = 80;
> +	char message[size];
> +	int len, ret = 0;

Indentation?

> +
> +	len = snprintf(message, size, "mark %s", mark);
> +	if (len >= size) {
> +		printf("mark '%s' is too long\n", mark);
> +		return ret;
> +	}
> +
> +	if (!(dmt = dm_task_create(DM_DEVICE_TARGET_MSG)))
> +		return ret;
> +
> +	if (!dm_task_set_name(dmt, device))
> +		goto out;
> +
> +	if (!dm_task_set_sector(dmt, 0))
> +		goto out;
> +
> +	if (!dm_task_set_message(dmt, message))
> +		goto out;
> +
> +	if (dm_task_run(dmt))
> +		ret = 1;
> +out:
> +	dm_task_destroy(dmt);
> +	return ret;
> +}
> +
> +static int
> +log_writes_f(
> +	int			argc,
> +	char			**argv)
> +{
> +	const char *device = NULL;
> +	const char *mark = NULL;

	const char		*device = NULL;
	const char		*mark = NULL;

Yeah, variable declaration indentation in the functions was the only
thing I found to complain about...

Otherwise looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> +	int c;
> +
> +	exitcode = 1;
> +	while ((c = getopt(argc, argv, "d:m:")) != EOF) {
> +		switch (c) {
> +		case 'd':
> +			device = optarg;
> +			break;
> +		case 'm':
> +			mark = optarg;
> +			break;
> +		default:
> +			return command_usage(&log_writes_cmd);
> +		}
> +	}
> +
> +	if (device == NULL || mark == NULL)
> +		return command_usage(&log_writes_cmd);
> +
> +	if (mark_log(device, mark))
> +		exitcode = 0;
> +
> +	return 0;
> +}
> +
> +void
> +log_writes_init(void)
> +{
> +	log_writes_cmd.name = "log_writes";
> +	log_writes_cmd.altname = "lw";
> +	log_writes_cmd.cfunc = log_writes_f;
> +	log_writes_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK
> +				| CMD_FLAG_ONESHOT;
> +	log_writes_cmd.argmin = 0;
> +	log_writes_cmd.argmax = -1;
> +	log_writes_cmd.args = _("-d device -m mark");
> +	log_writes_cmd.oneline =
> +		_("create mark <mark> in the dm-log-writes log specified by <device>");
> +
> +	add_command(&log_writes_cmd);
> +}
> diff --git a/m4/Makefile b/m4/Makefile
> index 4706121b..77f2edda 100644
> --- a/m4/Makefile
> +++ b/m4/Makefile
> @@ -15,6 +15,7 @@ CONFIGURE = \
>  LSRCFILES = \
>  	manual_format.m4 \
>  	package_blkid.m4 \
> +	package_devmapper.m4 \
>  	package_globals.m4 \
>  	package_libcdev.m4 \
>  	package_pthread.m4 \
> diff --git a/m4/package_devmapper.m4 b/m4/package_devmapper.m4
> new file mode 100644
> index 00000000..821f283c
> --- /dev/null
> +++ b/m4/package_devmapper.m4
> @@ -0,0 +1,11 @@
> +#
> +# See if libdevmapper is available on the system.
> +#
> +AC_DEFUN([AC_HAVE_DEVMAPPER],
> +[ AC_SEARCH_LIBS([dm_task_create], [devmapper],
> +        libdevmapper="-ldevmapper"
> +        have_devmapper=yes,
> +        have_devmapper=no,)
> +    AC_SUBST(have_devmapper)
> +    AC_SUBST(libdevmapper)
> +])
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 1693f7f1..4fce6d39 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -1123,7 +1123,27 @@ version of policy structure (numeric)
>  .BR get_encpolicy
>  On filesystems that support encryption, display the encryption policy of the
>  current file.
> +.RE
> +.PD
> +.TP
> +.BI "log_writes \-d " device " \-m "  mark
> +Create a mark named
> +.I mark
> +in the dm-log-writes log specified by
> +.I device.
> +This is intended to be equivalent to the shell command:
>  
> +.B dmsetup message
> +.I device
> +.B 0 mark
> +.I mark
> +.PD
> +.RE
> +.TP
> +.B lw
> +See the
> +.B log_writes
> +command.
>  .SH SEE ALSO
>  .BR mkfs.xfs (8),
>  .BR xfsctl (3),
> @@ -1141,4 +1161,5 @@ current file.
>  .BR open (2),
>  .BR pread (2),
>  .BR pwrite (2),
> -.BR readdir (3).
> +.BR readdir (3),
> +.BR dmsetup (8).
> -- 
> 2.14.3
> 
> --
> 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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
  2017-12-21 17:09     ` Darrick J. Wong
@ 2017-12-21 17:41       ` Ross Zwisler
  -1 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-21 17:41 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Thu, Dec 21, 2017 at 09:09:08AM -0800, Darrick J. Wong wrote:
> On Tue, Dec 05, 2017 at 04:56:50PM -0700, Ross Zwisler wrote:

> > @@ -195,6 +200,13 @@ mmap_f(
> >  		case 'x':
> >  			prot |= PROT_EXEC;
> >  			break;
> > +		case 'S':
> > +			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> > +			if (!flags) {
> 
> Heh, subtle. :)
> 
> /me wonders if it'd be better to do this explicitly:
> 
> #ifdef HAVE_MAP_SYNC
> 	flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> #else
> 	printf("MAP_SYNC not supported\n");
> 	return 1;
> #endif
> 
> ...though it's ugly.

Yea, I was trying to avoid #ifdefery.  If you prefer this, though, I'm happy
to change it.  Or maybe a comment would be enough?

/*
 * If MAP_SYNC and MAP_SHARED_VALIDATE aren't defined in the system headers we
 * will have defined them both as 0.
 */

?

> > +				printf("MAP_SYNC not supported\n");
> > +				return 0;
> 
> Are we supposed to be returning nonzero values for failing commands?

I don't think so.  All the other error conditions in this function also return
0.  I think the important thing is that 'exitcode' is set to 1 at the
beginning of the function per Dave's patch,

https://www.spinics.net/lists/linux-xfs/msg13605.html

which I pulled into my baseline:

https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/refs/?h=map_sync_v3

So, I likewise leave 'exitcode' as 1, bail out with a return code of 0, and
then you get the overall failure return of 1 from xfs_io at the shell.

> > @@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
> >  
> >  .SH MEMORY MAPPED I/O COMMANDS
> >  .TP
> > -.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
> > +.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
> >  With no arguments,
> >  .B mmap
> >  shows the current mappings. Specifying a single numeric argument
> > @@ -780,6 +780,10 @@ PROT_WRITE
> >  .RB ( \-w ),
> >  and PROT_EXEC
> >  .RB ( \-x ).
> > +The mapping will be created with the MAP_SHARED flag by default, or with the
> > +Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if
> 
> I assume I'll be able to look up exactly what MAP_SYNC provides in the mmap
> manpage, right?

Yep, Jan updated the man page for both MAP_SYNC and MAP_SHARED_VALIDATE:
https://lists.01.org/pipermail/linux-nvdimm/2017-November/013158.html

Thank you for the review.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
@ 2017-12-21 17:41       ` Ross Zwisler
  0 siblings, 0 replies; 42+ messages in thread
From: Ross Zwisler @ 2017-12-21 17:41 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Ross Zwisler, linux-xfs, linux-nvdimm, fstests, Jan Kara,
	Dave Chinner, Dan Williams

On Thu, Dec 21, 2017 at 09:09:08AM -0800, Darrick J. Wong wrote:
> On Tue, Dec 05, 2017 at 04:56:50PM -0700, Ross Zwisler wrote:

> > @@ -195,6 +200,13 @@ mmap_f(
> >  		case 'x':
> >  			prot |= PROT_EXEC;
> >  			break;
> > +		case 'S':
> > +			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> > +			if (!flags) {
> 
> Heh, subtle. :)
> 
> /me wonders if it'd be better to do this explicitly:
> 
> #ifdef HAVE_MAP_SYNC
> 	flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> #else
> 	printf("MAP_SYNC not supported\n");
> 	return 1;
> #endif
> 
> ...though it's ugly.

Yea, I was trying to avoid #ifdefery.  If you prefer this, though, I'm happy
to change it.  Or maybe a comment would be enough?

/*
 * If MAP_SYNC and MAP_SHARED_VALIDATE aren't defined in the system headers we
 * will have defined them both as 0.
 */

?

> > +				printf("MAP_SYNC not supported\n");
> > +				return 0;
> 
> Are we supposed to be returning nonzero values for failing commands?

I don't think so.  All the other error conditions in this function also return
0.  I think the important thing is that 'exitcode' is set to 1 at the
beginning of the function per Dave's patch,

https://www.spinics.net/lists/linux-xfs/msg13605.html

which I pulled into my baseline:

https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/refs/?h=map_sync_v3

So, I likewise leave 'exitcode' as 1, bail out with a return code of 0, and
then you get the overall failure return of 1 from xfs_io at the shell.

> > @@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
> >  
> >  .SH MEMORY MAPPED I/O COMMANDS
> >  .TP
> > -.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
> > +.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
> >  With no arguments,
> >  .B mmap
> >  shows the current mappings. Specifying a single numeric argument
> > @@ -780,6 +780,10 @@ PROT_WRITE
> >  .RB ( \-w ),
> >  and PROT_EXEC
> >  .RB ( \-x ).
> > +The mapping will be created with the MAP_SHARED flag by default, or with the
> > +Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if
> 
> I assume I'll be able to look up exactly what MAP_SYNC provides in the mmap
> manpage, right?

Yep, Jan updated the man page for both MAP_SYNC and MAP_SHARED_VALIDATE:
https://lists.01.org/pipermail/linux-nvdimm/2017-November/013158.html

Thank you for the review.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
  2017-12-21 17:41       ` Ross Zwisler
@ 2017-12-21 17:46         ` Darrick J. Wong
  -1 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-21 17:46 UTC (permalink / raw)
  To: Ross Zwisler; +Cc: Jan Kara, linux-nvdimm, Dave Chinner, fstests, linux-xfs

On Thu, Dec 21, 2017 at 10:41:10AM -0700, Ross Zwisler wrote:
> On Thu, Dec 21, 2017 at 09:09:08AM -0800, Darrick J. Wong wrote:
> > On Tue, Dec 05, 2017 at 04:56:50PM -0700, Ross Zwisler wrote:
> 
> > > @@ -195,6 +200,13 @@ mmap_f(
> > >  		case 'x':
> > >  			prot |= PROT_EXEC;
> > >  			break;
> > > +		case 'S':
> > > +			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> > > +			if (!flags) {
> > 
> > Heh, subtle. :)
> > 
> > /me wonders if it'd be better to do this explicitly:
> > 
> > #ifdef HAVE_MAP_SYNC
> > 	flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> > #else
> > 	printf("MAP_SYNC not supported\n");
> > 	return 1;
> > #endif
> > 
> > ...though it's ugly.
> 
> Yea, I was trying to avoid #ifdefery.  If you prefer this, though, I'm happy
> to change it.  Or maybe a comment would be enough?
> 
> /*
>  * If MAP_SYNC and MAP_SHARED_VALIDATE aren't defined in the system headers we
>  * will have defined them both as 0.
>  */
> 
> ?

Works for me.

> 
> > > +				printf("MAP_SYNC not supported\n");
> > > +				return 0;
> > 
> > Are we supposed to be returning nonzero values for failing commands?
> 
> I don't think so.  All the other error conditions in this function also return
> 0.  I think the important thing is that 'exitcode' is set to 1 at the
> beginning of the function per Dave's patch,
> 
> https://www.spinics.net/lists/linux-xfs/msg13605.html
> 
> which I pulled into my baseline:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/refs/?h=map_sync_v3
> 
> So, I likewise leave 'exitcode' as 1, bail out with a return code of 0, and
> then you get the overall failure return of 1 from xfs_io at the shell.

Ok.

> > > @@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
> > >  
> > >  .SH MEMORY MAPPED I/O COMMANDS
> > >  .TP
> > > -.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
> > > +.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
> > >  With no arguments,
> > >  .B mmap
> > >  shows the current mappings. Specifying a single numeric argument
> > > @@ -780,6 +780,10 @@ PROT_WRITE
> > >  .RB ( \-w ),
> > >  and PROT_EXEC
> > >  .RB ( \-x ).
> > > +The mapping will be created with the MAP_SHARED flag by default, or with the
> > > +Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if
> > 
> > I assume I'll be able to look up exactly what MAP_SYNC provides in the mmap
> > manpage, right?
> 
> Yep, Jan updated the man page for both MAP_SYNC and MAP_SHARED_VALIDATE:
> https://lists.01.org/pipermail/linux-nvdimm/2017-November/013158.html

Will have a look.

> Thank you for the review.

--D

> --
> 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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap()
@ 2017-12-21 17:46         ` Darrick J. Wong
  0 siblings, 0 replies; 42+ messages in thread
From: Darrick J. Wong @ 2017-12-21 17:46 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: linux-xfs, linux-nvdimm, fstests, Jan Kara, Dave Chinner, Dan Williams

On Thu, Dec 21, 2017 at 10:41:10AM -0700, Ross Zwisler wrote:
> On Thu, Dec 21, 2017 at 09:09:08AM -0800, Darrick J. Wong wrote:
> > On Tue, Dec 05, 2017 at 04:56:50PM -0700, Ross Zwisler wrote:
> 
> > > @@ -195,6 +200,13 @@ mmap_f(
> > >  		case 'x':
> > >  			prot |= PROT_EXEC;
> > >  			break;
> > > +		case 'S':
> > > +			flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> > > +			if (!flags) {
> > 
> > Heh, subtle. :)
> > 
> > /me wonders if it'd be better to do this explicitly:
> > 
> > #ifdef HAVE_MAP_SYNC
> > 	flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> > #else
> > 	printf("MAP_SYNC not supported\n");
> > 	return 1;
> > #endif
> > 
> > ...though it's ugly.
> 
> Yea, I was trying to avoid #ifdefery.  If you prefer this, though, I'm happy
> to change it.  Or maybe a comment would be enough?
> 
> /*
>  * If MAP_SYNC and MAP_SHARED_VALIDATE aren't defined in the system headers we
>  * will have defined them both as 0.
>  */
> 
> ?

Works for me.

> 
> > > +				printf("MAP_SYNC not supported\n");
> > > +				return 0;
> > 
> > Are we supposed to be returning nonzero values for failing commands?
> 
> I don't think so.  All the other error conditions in this function also return
> 0.  I think the important thing is that 'exitcode' is set to 1 at the
> beginning of the function per Dave's patch,
> 
> https://www.spinics.net/lists/linux-xfs/msg13605.html
> 
> which I pulled into my baseline:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/zwisler/xfsprogs-dev.git/refs/?h=map_sync_v3
> 
> So, I likewise leave 'exitcode' as 1, bail out with a return code of 0, and
> then you get the overall failure return of 1 from xfs_io at the shell.

Ok.

> > > @@ -764,7 +764,7 @@ Each (sec, nsec) pair constitutes a single timestamp value.
> > >  
> > >  .SH MEMORY MAPPED I/O COMMANDS
> > >  .TP
> > > -.BI "mmap [ " N " | [[ \-rwx ] [\-s " size " ] " "offset length " ]]
> > > +.BI "mmap [ " N " | [[ \-rwxS ] [\-s " size " ] " "offset length " ]]
> > >  With no arguments,
> > >  .B mmap
> > >  shows the current mappings. Specifying a single numeric argument
> > > @@ -780,6 +780,10 @@ PROT_WRITE
> > >  .RB ( \-w ),
> > >  and PROT_EXEC
> > >  .RB ( \-x ).
> > > +The mapping will be created with the MAP_SHARED flag by default, or with the
> > > +Linux specific (MAP_SYNC | MAP_SHARED_VALIDATE) flags if
> > 
> > I assume I'll be able to look up exactly what MAP_SYNC provides in the mmap
> > manpage, right?
> 
> Yep, Jan updated the man page for both MAP_SYNC and MAP_SHARED_VALIDATE:
> https://lists.01.org/pipermail/linux-nvdimm/2017-November/013158.html

Will have a look.

> Thank you for the review.

--D

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2017-12-21 17:49 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-05 23:56 [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing Ross Zwisler
2017-12-05 23:56 ` Ross Zwisler
2017-12-05 23:56 ` [xfsprogs PATCH v2 1/3] xfs_io: fix compiler warnings in getfsmap code Ross Zwisler
2017-12-05 23:56   ` Ross Zwisler
2017-12-06  0:03   ` Darrick J. Wong
2017-12-06  0:03     ` Darrick J. Wong
2017-12-06  0:27   ` Dave Chinner
2017-12-06  0:27     ` Dave Chinner
2017-12-06 13:59     ` Eric Sandeen
2017-12-06 13:59       ` Eric Sandeen
2017-12-06 20:10     ` Ross Zwisler
2017-12-06 20:10       ` Ross Zwisler
2017-12-06 20:47       ` Darrick J. Wong
2017-12-06 20:47         ` Darrick J. Wong
2017-12-06 20:58         ` Ross Zwisler
2017-12-06 20:58           ` Ross Zwisler
2017-12-05 23:56 ` [xfsprogs PATCH v2 2/3] xfs_io: add MAP_SYNC support to mmap() Ross Zwisler
2017-12-05 23:56   ` Ross Zwisler
2017-12-21 17:09   ` Darrick J. Wong
2017-12-21 17:09     ` Darrick J. Wong
2017-12-21 17:41     ` Ross Zwisler
2017-12-21 17:41       ` Ross Zwisler
2017-12-21 17:46       ` Darrick J. Wong
2017-12-21 17:46         ` Darrick J. Wong
2017-12-05 23:56 ` [xfsprogs PATCH v2 3/3] xfs_io: add a new 'log_writes' command Ross Zwisler
2017-12-05 23:56   ` Ross Zwisler
2017-12-06  0:29   ` Dave Chinner
2017-12-06  0:29     ` Dave Chinner
2017-12-06  4:38     ` Ross Zwisler
2017-12-06  4:38       ` Ross Zwisler
2017-12-06  4:41       ` Ross Zwisler
2017-12-06  4:41         ` Ross Zwisler
2017-12-06  5:43       ` Dave Chinner
2017-12-06  5:43         ` Dave Chinner
2017-12-06 18:13   ` [xfsprogs PATCH v3 " Ross Zwisler
2017-12-06 18:13     ` Ross Zwisler
2017-12-21 17:14     ` Darrick J. Wong
2017-12-21 17:14       ` Darrick J. Wong
2017-12-13 16:45 ` [xfsprogs PATCH v2 0/3] Add necessary items for MAP_SYNC testing Ross Zwisler
2017-12-13 16:45   ` Ross Zwisler
2017-12-21 16:55   ` Ross Zwisler
2017-12-21 16:55     ` Ross Zwisler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.