pmemblk and dev-dax cleanups

pmemblk and dev-dax cleanups

[Robert Elliott]

This series changes library loading to startup time, and resubmits
pmemblk and dev-dax patches that were not posted earlier but not
included in the previous 12-part series of binary/decimal prefix
cleanups.

 [PATCH 1/4] pmemblk, dev-dax: load libpmem and libpmemblk at startup
 [PATCH 2/4] pmemblk, dev-dax: Update descriptions
 [PATCH 3/4] pmemblk,dev-dax: clean up error logs
 [PATCH 4/4] pmemblk: Clarify fsize is in MiB not MB

 HOWTO             |  10 +++++---
 configure         |   4 ++-
 engines/dev-dax.c |  44 +++++++++-----------------------
 engines/pmemblk.c | 117 ++++++++++++++++++++++--------------------------------------------------------------
 fio.1             |   6 +++--
 5 files changed, 56 insertions(+), 125 deletions(-)

[PATCH 1/4] pmemblk, dev-dax: load libpmem and libpmemblk at startup

[Robert Elliott]

From: Robert Elliott <elliott@hpe.com>

The pmemblk and dev-dax ioengines were loading libpmem.so and
libpmemblk.so using dlopen() at runtime.

Although the upstream nvml Makefile installs a libpmem.so
symbolic link, some of the distros (e.g. SUSE Linux Enterprise
Server 12 SP2) are just installing so.1 symbolic links.  So,
fio fails to find the libpmem.so and libpmemblk.so library files.

http://www.ibm.com/developerworks/linux/library/l-shlibs/index.html
says applications should always load the versioned filenames to
avoid compatibility issues; the non-versioned filenames are just
for the compiler and linker.

Change ./configure to pass -lpmem and -lpmemblk to the compiler
so the fio binary loads the versioned filename at startup like
the other libraries, rather than open the non-versioned filename
with dlopen() during runtime.

This way they show up in ldd:
$ ldd fio
        linux-vdso.so.1 (0x00007ffc290aa000)
        libpmemblk.so.1 => /lib64/libpmemblk.so.1 (0x00007f65b5bc4000)
        libpmem.so.1 => /lib64/libpmem.so.1 (0x00007f65b59be000)
        libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f65b57b3000)
        librt.so.1 => /lib64/librt.so.1 (0x00007f65b55ab000)
        libaio.so.1 => /lib64/libaio.so.1 (0x00007f65b53a9000)
        libz.so.1 => /lib64/libz.so.1 (0x00007f65b5191000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f65b4e88000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f65b4c6a000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00007f65b4a66000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f65b46a0000)
        libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f65b449b000)
        /lib64/ld-linux-x86-64.so.2 (0x0000559f4d872000)
---
 configure         |  4 ++-
 engines/dev-dax.c | 22 +---------------
 engines/pmemblk.c | 77 +++++--------------------------------------------------
 3 files changed, 11 insertions(+), 92 deletions(-)

diff --git a/configure b/configure
index fc15782..ca69052 100755
--- a/configure
+++ b/configure
@@ -1573,11 +1573,12 @@ int main(int argc, char **argv)
 {
   int rc;
   rc = pmem_is_pmem(0, 0);
-  return 0;
+  return rc;
 }
 EOF
 if compile_prog "" "-lpmem" "libpmem"; then
   libpmem="yes"
+  LIBS="-lpmem $LIBS"
 fi
 echo "libpmem                       $libpmem"
 
@@ -1595,6 +1596,7 @@ int main(int argc, char **argv)
 EOF
 if compile_prog "" "-lpmemblk -lpmem" "libpmemblk"; then
   libpmemblk="yes"
+  LIBS="-lpmemblk -lpmem $LIBS"
 fi
 echo "libpmemblk                    $libpmemblk"
 
diff --git a/engines/dev-dax.c b/engines/dev-dax.c
index 2516bca..45aca4e 100644
--- a/engines/dev-dax.c
+++ b/engines/dev-dax.c
@@ -51,8 +51,8 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/sysmacros.h>
-#include <dlfcn.h>
 #include <libgen.h>
+#include <libpmem.h>
 
 #include "../fio.h"
 #include "../verify.h"
@@ -69,8 +69,6 @@ struct fio_devdax_data {
 	off_t devdax_off;
 };
 
-static void * (*pmem_memcpy_persist)(void *dest, const void *src, size_t len);
-
 static int fio_devdax_file(struct thread_data *td, struct fio_file *f,
 			   size_t length, off_t off)
 {
@@ -212,8 +210,6 @@ static int fio_devdax_queue(struct thread_data *td, struct io_u *io_u)
 static int fio_devdax_init(struct thread_data *td)
 {
 	struct thread_options *o = &td->o;
-	const char *path;
-	void *dl;
 
 	if ((o->rw_min_bs & page_mask) &&
 	    (o->fsync_blocks || o->fdatasync_blocks)) {
@@ -222,22 +218,6 @@ static int fio_devdax_init(struct thread_data *td)
 		return 1;
 	}
 
-	path = getenv("FIO_PMEM_LIB");
-	if (!path)
-		path = "libpmem.so";
-
-	dl = dlopen(path, RTLD_NOW | RTLD_NODELETE);
-	if (!dl) {
-		log_err("fio: unable to open libpmem: %s\n", dlerror());
-		return 1;
-	}
-
-	pmem_memcpy_persist = dlsym(dl, "pmem_memcpy_persist");
-	if (!pmem_memcpy_persist) {
-		log_err("fio: unable to load libpmem: %s\n", dlerror());
-		return 1;
-	}
-
 	return 0;
 }
 
diff --git a/engines/pmemblk.c b/engines/pmemblk.c
index ca72697..7b1affa 100644
--- a/engines/pmemblk.c
+++ b/engines/pmemblk.c
@@ -50,12 +50,6 @@
  *
  *   See examples/pmemblk.fio for more.
  *
- * libpmemblk.so
- *   By default, the pmemblk engine will let the system find the libpmemblk.so
- *   that it uses.  You can use an alternative libpmemblk by setting the
- *   FIO_PMEMBLK_LIB environment variable to the full path to the desired
- *   libpmemblk.so.
- *
  */
 
 #include <stdio.h>
@@ -64,68 +58,15 @@
 #include <sys/uio.h>
 #include <errno.h>
 #include <assert.h>
-#include <dlfcn.h>
 #include <string.h>
+#include <libpmem.h>
+#include <libpmemblk.h>
 
 #include "../fio.h"
 
 /*
  * libpmemblk
  */
-struct PMEMblkpool_s;
-typedef struct PMEMblkpool_s PMEMblkpool;
-
-static PMEMblkpool *(*pmemblk_create) (const char *, size_t, size_t, mode_t);
-static PMEMblkpool *(*pmemblk_open) (const char *, size_t);
-static void (*pmemblk_close) (PMEMblkpool *);
-static size_t(*pmemblk_nblock) (PMEMblkpool *);
-static size_t(*pmemblk_bsize) (PMEMblkpool *);
-static int (*pmemblk_read) (PMEMblkpool *, void *, off_t);
-static int (*pmemblk_write) (PMEMblkpool *, const void *, off_t);
-
-int load_libpmemblk(const char *path)
-{
-	void *dl;
-
-	if (!path)
-		path = "libpmemblk.so";
-
-	dl = dlopen(path, RTLD_NOW | RTLD_NODELETE);
-	if (!dl)
-		goto errorout;
-
-	pmemblk_create = dlsym(dl, "pmemblk_create");
-	if (!pmemblk_create)
-		goto errorout;
-	pmemblk_open = dlsym(dl, "pmemblk_open");
-	if (!pmemblk_open)
-		goto errorout;
-	pmemblk_close = dlsym(dl, "pmemblk_close");
-	if (!pmemblk_close)
-		goto errorout;
-	pmemblk_nblock = dlsym(dl, "pmemblk_nblock");
-	if (!pmemblk_nblock)
-		goto errorout;
-	pmemblk_bsize = dlsym(dl, "pmemblk_bsize");
-	if (!pmemblk_bsize)
-		goto errorout;
-	pmemblk_read = dlsym(dl, "pmemblk_read");
-	if (!pmemblk_read)
-		goto errorout;
-	pmemblk_write = dlsym(dl, "pmemblk_write");
-	if (!pmemblk_write)
-		goto errorout;
-
-	return 0;
-
-errorout:
-	log_err("fio: unable to load libpmemblk: %s\n", dlerror());
-	if (dl)
-		dlclose(dl);
-
-	return -1;
-}
-
 typedef struct fio_pmemblk_file *fio_pmemblk_file_t;
 
 struct fio_pmemblk_file {
@@ -250,11 +191,6 @@ static fio_pmemblk_file_t pmb_open(const char *pathspec, int flags)
 
 	pmb = fio_pmemblk_cache_lookup(path);
 	if (!pmb) {
-		/* load libpmemblk if needed */
-		if (!pmemblk_open)
-			if (load_libpmemblk(getenv("FIO_PMEMBLK_LIB")))
-				goto error;
-
 		pmb = malloc(sizeof(*pmb));
 		if (!pmb)
 			goto error;
@@ -410,14 +346,11 @@ static int fio_pmemblk_queue(struct thread_data *td, struct io_u *io_u)
 	unsigned long long off;
 	unsigned long len;
 	void *buf;
-	int (*blkop) (PMEMblkpool *, void *, off_t) = (void *)pmemblk_write;
 
 	fio_ro_check(td, io_u);
 
 	switch (io_u->ddir) {
 	case DDIR_READ:
-		blkop = pmemblk_read;
-		/* fall through */
 	case DDIR_WRITE:
 		off = io_u->offset;
 		len = io_u->xfer_buflen;
@@ -435,7 +368,11 @@ static int fio_pmemblk_queue(struct thread_data *td, struct io_u *io_u)
 		off /= pmb->pmb_bsize;
 		len /= pmb->pmb_bsize;
 		while (0 < len) {
-			if (0 != blkop(pmb->pmb_pool, buf, off)) {
+			if (io_u->ddir == DDIR_READ &&
+			   0 != pmemblk_read(pmb->pmb_pool, buf, off)) {
+				io_u->error = errno;
+				break;
+			} else if (0 != pmemblk_write(pmb->pmb_pool, buf, off)) {
 				io_u->error = errno;
 				break;
 			}
-- 
2.9.3

[PATCH 2/4] pmemblk, dev-dax: Update descriptions

[Robert Elliott]

From: Robert Elliott <elliott@hpe.com>

Make the descriptions of the pmemblk and dev-dax ioengines
more symmetric.
---
 HOWTO | 10 ++++++----
 fio.1 |  6 ++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/HOWTO b/HOWTO
index 4354e46..9e1ffa6 100644
--- a/HOWTO
+++ b/HOWTO
@@ -903,11 +903,13 @@ ioengine=str	Defines how the job issues io to the file. The following
 				overwriting. The writetrim mode works well
 				for this constraint.
 
-			pmemblk	Read and write through the NVML libpmemblk
-				interface.
+			pmemblk	Read and write using filesystem DAX to a file
+				on a filesystem mounted with DAX on a persistent
+				memory device through the NVML libpmemblk library.
 
-			dev-dax Read and write through a DAX device exposed
-				from persistent memory.
+			dev-dax Read and write using device DAX to a persistent
+				memory device (e.g., /dev/dax0.0) through the
+				NVML libpmem library.
 
 			external Prefix to specify loading an external
 				IO engine object file. Append the engine
diff --git a/fio.1 b/fio.1
index f486276..56f2d11 100644
--- a/fio.1
+++ b/fio.1
@@ -811,10 +811,12 @@ and discarding before overwriting. The trimwrite mode works well for this
 constraint.
 .TP
 .B pmemblk
-Read and write through the NVML libpmemblk interface.
+Read and write using filesystem DAX to a file on a filesystem mounted with
+DAX on a persistent memory device through the NVML libpmemblk library.
 .TP
 .B dev-dax
-Read and write through a DAX device exposed from persistent memory.
+Read and write using device DAX to a persistent memory device
+(e.g., /dev/dax0.0) through the NVML libpmem library.
 .RE
 .P
 .RE
-- 
2.9.3

[PATCH 3/4] pmemblk,dev-dax: clean up error logs

[Robert Elliott]

Print the errno string rather than the number, and prefix
all log_err prints with pmemblk or dev-dax rather than fio.
---
 engines/dev-dax.c | 22 +++++++++++-----------
 engines/pmemblk.c |  9 ++++-----
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/engines/dev-dax.c b/engines/dev-dax.c
index 45aca4e..235a31e 100644
--- a/engines/dev-dax.c
+++ b/engines/dev-dax.c
@@ -106,7 +106,7 @@ static int fio_devdax_prep_limited(struct thread_data *td, struct io_u *io_u)
 	struct fio_devdax_data *fdd = FILE_ENG_DATA(f);
 
 	if (io_u->buflen > f->real_file_size) {
-		log_err("fio: bs too big for dev-dax engine\n");
+		log_err("dev-dax: bs too big for dev-dax engine\n");
 		return EIO;
 	}
 
@@ -213,8 +213,8 @@ static int fio_devdax_init(struct thread_data *td)
 
 	if ((o->rw_min_bs & page_mask) &&
 	    (o->fsync_blocks || o->fdatasync_blocks)) {
-		log_err("fio: mmap options dictate a minimum block size of "
-			"%llu bytes\n", (unsigned long long) page_size);
+		log_err("dev-dax: mmap options dictate a minimum block size of %llu bytes\n",
+			(unsigned long long) page_size);
 		return 1;
 	}
 
@@ -272,8 +272,8 @@ fio_devdax_get_file_size(struct thread_data *td, struct fio_file *f)
 
 	rc = stat(f->file_name, &st);
 	if (rc < 0) {
-		log_err("%s: failed to stat file %s: %d\n",
-			td->o.name, f->file_name, errno);
+		log_err("%s: failed to stat file %s (%s)\n",
+			td->o.name, f->file_name, strerror(errno));
 		return -errno;
 	}
 
@@ -282,8 +282,8 @@ fio_devdax_get_file_size(struct thread_data *td, struct fio_file *f)
 
 	rpath = realpath(spath, npath);
 	if (!rpath) {
-		log_err("%s: realpath on %s failed: %d\n",
-			td->o.name, spath, errno);
+		log_err("%s: realpath on %s failed (%s)\n",
+			td->o.name, spath, strerror(errno));
 		return -errno;
 	}
 
@@ -298,15 +298,15 @@ fio_devdax_get_file_size(struct thread_data *td, struct fio_file *f)
 
 	sfile = fopen(spath, "r");
 	if (!sfile) {
-		log_err("%s: fopen on %s failed: %d\n",
-			td->o.name, spath, errno);
+		log_err("%s: fopen on %s failed (%s)\n",
+			td->o.name, spath, strerror(errno));
 		return 1;
 	}
 
 	rc = fscanf(sfile, "%lu", &size);
 	if (rc < 0) {
-		log_err("%s: fscanf on %s failed: %d\n",
-			td->o.name, spath, errno);
+		log_err("%s: fscanf on %s failed (%s)\n",
+			td->o.name, spath, strerror(errno));
 		return 1;
 	}
 
diff --git a/engines/pmemblk.c b/engines/pmemblk.c
index 7b1affa..7c609cf 100644
--- a/engines/pmemblk.c
+++ b/engines/pmemblk.c
@@ -203,9 +203,8 @@ static fio_pmemblk_file_t pmb_open(const char *pathspec, int flags)
 			    pmemblk_create(path, bsize, fsize, 0644);
 		}
 		if (!pmb->pmb_pool) {
-			log_err
-			    ("fio: enable to open pmemblk pool file (errno %d)\n",
-			     errno);
+			log_err("pmemblk: unable to open pmemblk pool file %s (%s)\n",
+			     path, strerror(errno));
 			goto error;
 		}
 
@@ -267,14 +266,14 @@ static int pmb_get_flags(struct thread_data *td, uint64_t *pflags)
 	if (!td->o.use_thread) {
 		if (!thread_warned) {
 			thread_warned = 1;
-			log_err("fio: must set thread=1 for pmemblk engine\n");
+			log_err("pmemblk: must set thread=1 for pmemblk engine\n");
 		}
 		return 1;
 	}
 
 	if (!td->o.odirect && !odirect_warned) {
 		odirect_warned = 1;
-		log_info("fio: direct == 0, but pmemblk is always direct\n");
+		log_info("pmemblk: direct == 0, but pmemblk is always direct\n");
 	}
 
 	if (td->o.allow_create)
-- 
2.9.3

[PATCH 4/4] pmemblk: Clarify fsize is in MiB not MB

[Robert Elliott]

From: Robert Elliott <elliott@hpe.com>

Clarify that the FSIZE argument is in MiB (power of 2) rather
than MB (power of 10) in comments and variable names.
---
 engines/pmemblk.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/engines/pmemblk.c b/engines/pmemblk.c
index 7c609cf..c70015b 100644
--- a/engines/pmemblk.c
+++ b/engines/pmemblk.c
@@ -27,11 +27,11 @@
  *   ioengine=pmemblk
  *
  * Other relevant settings:
+ *   thread=1   REQUIRED
  *   iodepth=1
  *   direct=1
- *   thread=1   REQUIRED
  *   unlink=1
- *   filename=/pmem0/fiotestfile,BSIZE,FSIZEMB
+ *   filename=/mnt/pmem0/fiotestfile,BSIZE,FSIZEMiB
  *
  *   thread must be set to 1 for pmemblk as multiple processes cannot
  *     open the same block pool file.
@@ -39,14 +39,23 @@
  *   iodepth should be set to 1 as pmemblk is always synchronous.
  *   Use numjobs to scale up.
  *
- *   direct=1 is implied as pmemblk is always direct.
+ *   direct=1 is implied as pmemblk is always direct. A warning message
+ *   is printed if this is not specified.
+ *
+ *   unlink=1 removes the block pool file after testing, and is optional.
+ *
+ *   The pmem device must have a DAX-capable filesystem and be mounted
+ *   with DAX enabled.  filename must point to a file on that filesystem.
  *
- *   Can set unlink to 1 to remove the block pool file after testing.
+ *   Example:
+ *     mkfs.xfs /dev/pmem0
+ *     mkdir /mnt/pmem0
+ *     mount -o dax /dev/pmem0 /mnt/pmem0
  *
  *   When specifying the filename, if the block pool file does not already
- *   exist, then the pmemblk engine can create the pool file if you specify
+ *   exist, then the pmemblk engine creates the pool file if you specify
  *   the block and file sizes.  BSIZE is the block size in bytes.
- *   FSIZEMB is the pool file size in MB.
+ *   FSIZEMB is the pool file size in MiB.
  *
  *   See examples/pmemblk.fio for more.
  *
@@ -128,7 +137,7 @@ static void fio_pmemblk_cache_remove(fio_pmemblk_file_t pmb)
  * level, we allow the block size and file size to be appended
  * to the file name:
  *
- *   path[,bsize,fsizemb]
+ *   path[,bsize,fsizemib]
  *
  * note that we do not use the fio option "filesize" to dictate
  * the file size because we can only give libpmemblk the gross
@@ -138,7 +147,7 @@ static void fio_pmemblk_cache_remove(fio_pmemblk_file_t pmb)
  * the final path without the parameters is returned in ppath.
  * the block size and file size are returned in pbsize and fsize.
  *
- * note that the user should specify the file size in MiB, but
+ * note that the user specifies the file size in MiB, but
  * we return bytes from here.
  */
 static void pmb_parse_path(const char *pathspec, char **ppath, uint64_t *pbsize,
@@ -147,7 +156,7 @@ static void pmb_parse_path(const char *pathspec, char **ppath, uint64_t *pbsize,
 	char *path;
 	char *s;
 	uint64_t bsize;
-	uint64_t fsizemb;
+	uint64_t fsizemib;
 
 	path = strdup(pathspec);
 	if (!path) {
@@ -157,14 +166,14 @@ static void pmb_parse_path(const char *pathspec, char **ppath, uint64_t *pbsize,
 
 	/* extract sizes, if given */
 	s = strrchr(path, ',');
-	if (s && (fsizemb = strtoull(s + 1, NULL, 10))) {
+	if (s && (fsizemib = strtoull(s + 1, NULL, 10))) {
 		*s = 0;
 		s = strrchr(path, ',');
 		if (s && (bsize = strtoull(s + 1, NULL, 10))) {
 			*s = 0;
 			*ppath = path;
 			*pbsize = bsize;
-			*pfsize = fsizemb << 20;
+			*pfsize = fsizemib << 20;
 			return;
 		}
 	}
-- 
2.9.3

RE: [PATCH 1/4] pmemblk, dev-dax: load libpmem and libpmemblk at startup

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: Elliott, Robert (Persistent Memory)
> Sent: Wednesday, January 04, 2017 12:55 AM
> Subject: [PATCH 1/4] pmemblk, dev-dax: load libpmem and libpmemblk at
> startup
> 
...
> diff --git a/configure b/configure
> index fc15782..ca69052 100755
> --- a/configure
> +++ b/configure
> @@ -1573,11 +1573,12 @@ int main(int argc, char **argv)
>  {
>    int rc;
>    rc = pmem_is_pmem(0, 0);
> -  return 0;
> +  return rc;

That change should not be applied - the return code is irrelevant
on the compiling machine (0 is the expected result since the len
being checked is zero). This is just used to check if a program
making that function call compiles or not.

Re: pmemblk and dev-dax cleanups

[Jens Axboe]

On 01/03/2017 11:54 PM, Robert Elliott wrote:
> This series changes library loading to startup time, and resubmits
> pmemblk and dev-dax patches that were not posted earlier but not
> included in the previous 12-part series of binary/decimal prefix
> cleanups.
> 
>  [PATCH 1/4] pmemblk, dev-dax: load libpmem and libpmemblk at startup
>  [PATCH 2/4] pmemblk, dev-dax: Update descriptions
>  [PATCH 3/4] pmemblk,dev-dax: clean up error logs
>  [PATCH 4/4] pmemblk: Clarify fsize is in MiB not MB

Will you resend with 1/4 fixed up?

-- 
Jens Axboe

[PATCH v2 1/4] pmemblk, dev-dax: load libpmem and libpmemblk at startup

[Robert Elliott]

From: Robert Elliott <elliott@hpe.com>

The pmemblk and dev-dax ioengines were loading libpmem.so and
libpmemblk.so using dlopen() at runtime.

Although the upstream nvml Makefile installs a libpmem.so
symbolic link, some of the distros (e.g. SUSE Linux Enterprise
Server 12 SP2) are just installing so.1 symbolic links.  So,
fio fails to find the libpmem.so and libpmemblk.so library files.

http://www.ibm.com/developerworks/linux/library/l-shlibs/index.html
says applications should always load the versioned filenames to
avoid compatibility issues; the non-versioned filenames are just
for the compiler and linker.

Change ./configure to pass -lpmem and -lpmemblk to the compiler
so the fio binary loads the versioned filename at startup like
the other libraries, rather than open the non-versioned filename
with dlopen() during runtime.

This way they show up in ldd:
$ ldd fio
        linux-vdso.so.1 (0x00007ffc290aa000)
        libpmemblk.so.1 => /lib64/libpmemblk.so.1 (0x00007f65b5bc4000)
        libpmem.so.1 => /lib64/libpmem.so.1 (0x00007f65b59be000)
        libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f65b57b3000)
        librt.so.1 => /lib64/librt.so.1 (0x00007f65b55ab000)
        libaio.so.1 => /lib64/libaio.so.1 (0x00007f65b53a9000)
        libz.so.1 => /lib64/libz.so.1 (0x00007f65b5191000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f65b4e88000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f65b4c6a000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00007f65b4a66000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f65b46a0000)
        libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f65b449b000)
        /lib64/ld-linux-x86-64.so.2 (0x0000559f4d872000)
---
 configure         | 20 ++++++++++-----
 engines/dev-dax.c | 22 +---------------
 engines/pmemblk.c | 77 +++++--------------------------------------------------
 3 files changed, 21 insertions(+), 98 deletions(-)

diff --git a/configure b/configure
index fc15782..68c4859 100755
--- a/configure
+++ b/configure
@@ -1578,26 +1578,32 @@ int main(int argc, char **argv)
 EOF
 if compile_prog "" "-lpmem" "libpmem"; then
   libpmem="yes"
+  LIBS="-lpmem $LIBS"
 fi
 echo "libpmem                       $libpmem"
 
 ##########################################
 # Check whether we have libpmemblk
+# libpmem is a prerequisite
 libpmemblk="no"
-cat > $TMPC << EOF
+if test "$libpmem" = "yes"; then
+  cat > $TMPC << EOF
 #include <libpmemblk.h>
 int main(int argc, char **argv)
 {
-  int rc;
-  rc = pmemblk_open("", 0);
+  PMEMblkpool *pbp;
+  pbp = pmemblk_open("", 0);
   return 0;
 }
 EOF
-if compile_prog "" "-lpmemblk -lpmem" "libpmemblk"; then
-  libpmemblk="yes"
+  if compile_prog "" "-lpmemblk" "libpmemblk"; then
+    libpmemblk="yes"
+    LIBS="-lpmemblk $LIBS"
+  fi
 fi
 echo "libpmemblk                    $libpmemblk"
 
+# Choose the ioengines
 if test "$libpmem" = "yes" && test "$disable_pmem" = "no"; then
   devdax="yes"
   if test "$libpmemblk" = "yes"; then
@@ -1607,11 +1613,11 @@ fi
 
 ##########################################
 # Report whether pmemblk engine is enabled
-echo "NVML libpmemblk engine        $pmemblk"
+echo "NVML pmemblk engine           $pmemblk"
 
 ##########################################
 # Report whether dev-dax engine is enabled
-echo "NVML Device Dax engine        $devdax"
+echo "NVML dev-dax engine           $devdax"
 
 # Check if we have lex/yacc available
 yacc="no"
diff --git a/engines/dev-dax.c b/engines/dev-dax.c
index 2516bca..45aca4e 100644
--- a/engines/dev-dax.c
+++ b/engines/dev-dax.c
@@ -51,8 +51,8 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/sysmacros.h>
-#include <dlfcn.h>
 #include <libgen.h>
+#include <libpmem.h>
 
 #include "../fio.h"
 #include "../verify.h"
@@ -69,8 +69,6 @@ struct fio_devdax_data {
 	off_t devdax_off;
 };
 
-static void * (*pmem_memcpy_persist)(void *dest, const void *src, size_t len);
-
 static int fio_devdax_file(struct thread_data *td, struct fio_file *f,
 			   size_t length, off_t off)
 {
@@ -212,8 +210,6 @@ static int fio_devdax_queue(struct thread_data *td, struct io_u *io_u)
 static int fio_devdax_init(struct thread_data *td)
 {
 	struct thread_options *o = &td->o;
-	const char *path;
-	void *dl;
 
 	if ((o->rw_min_bs & page_mask) &&
 	    (o->fsync_blocks || o->fdatasync_blocks)) {
@@ -222,22 +218,6 @@ static int fio_devdax_init(struct thread_data *td)
 		return 1;
 	}
 
-	path = getenv("FIO_PMEM_LIB");
-	if (!path)
-		path = "libpmem.so";
-
-	dl = dlopen(path, RTLD_NOW | RTLD_NODELETE);
-	if (!dl) {
-		log_err("fio: unable to open libpmem: %s\n", dlerror());
-		return 1;
-	}
-
-	pmem_memcpy_persist = dlsym(dl, "pmem_memcpy_persist");
-	if (!pmem_memcpy_persist) {
-		log_err("fio: unable to load libpmem: %s\n", dlerror());
-		return 1;
-	}
-
 	return 0;
 }
 
diff --git a/engines/pmemblk.c b/engines/pmemblk.c
index ca72697..7b1affa 100644
--- a/engines/pmemblk.c
+++ b/engines/pmemblk.c
@@ -50,12 +50,6 @@
  *
  *   See examples/pmemblk.fio for more.
  *
- * libpmemblk.so
- *   By default, the pmemblk engine will let the system find the libpmemblk.so
- *   that it uses.  You can use an alternative libpmemblk by setting the
- *   FIO_PMEMBLK_LIB environment variable to the full path to the desired
- *   libpmemblk.so.
- *
  */
 
 #include <stdio.h>
@@ -64,68 +58,15 @@
 #include <sys/uio.h>
 #include <errno.h>
 #include <assert.h>
-#include <dlfcn.h>
 #include <string.h>
+#include <libpmem.h>
+#include <libpmemblk.h>
 
 #include "../fio.h"
 
 /*
  * libpmemblk
  */
-struct PMEMblkpool_s;
-typedef struct PMEMblkpool_s PMEMblkpool;
-
-static PMEMblkpool *(*pmemblk_create) (const char *, size_t, size_t, mode_t);
-static PMEMblkpool *(*pmemblk_open) (const char *, size_t);
-static void (*pmemblk_close) (PMEMblkpool *);
-static size_t(*pmemblk_nblock) (PMEMblkpool *);
-static size_t(*pmemblk_bsize) (PMEMblkpool *);
-static int (*pmemblk_read) (PMEMblkpool *, void *, off_t);
-static int (*pmemblk_write) (PMEMblkpool *, const void *, off_t);
-
-int load_libpmemblk(const char *path)
-{
-	void *dl;
-
-	if (!path)
-		path = "libpmemblk.so";
-
-	dl = dlopen(path, RTLD_NOW | RTLD_NODELETE);
-	if (!dl)
-		goto errorout;
-
-	pmemblk_create = dlsym(dl, "pmemblk_create");
-	if (!pmemblk_create)
-		goto errorout;
-	pmemblk_open = dlsym(dl, "pmemblk_open");
-	if (!pmemblk_open)
-		goto errorout;
-	pmemblk_close = dlsym(dl, "pmemblk_close");
-	if (!pmemblk_close)
-		goto errorout;
-	pmemblk_nblock = dlsym(dl, "pmemblk_nblock");
-	if (!pmemblk_nblock)
-		goto errorout;
-	pmemblk_bsize = dlsym(dl, "pmemblk_bsize");
-	if (!pmemblk_bsize)
-		goto errorout;
-	pmemblk_read = dlsym(dl, "pmemblk_read");
-	if (!pmemblk_read)
-		goto errorout;
-	pmemblk_write = dlsym(dl, "pmemblk_write");
-	if (!pmemblk_write)
-		goto errorout;
-
-	return 0;
-
-errorout:
-	log_err("fio: unable to load libpmemblk: %s\n", dlerror());
-	if (dl)
-		dlclose(dl);
-
-	return -1;
-}
-
 typedef struct fio_pmemblk_file *fio_pmemblk_file_t;
 
 struct fio_pmemblk_file {
@@ -250,11 +191,6 @@ static fio_pmemblk_file_t pmb_open(const char *pathspec, int flags)
 
 	pmb = fio_pmemblk_cache_lookup(path);
 	if (!pmb) {
-		/* load libpmemblk if needed */
-		if (!pmemblk_open)
-			if (load_libpmemblk(getenv("FIO_PMEMBLK_LIB")))
-				goto error;
-
 		pmb = malloc(sizeof(*pmb));
 		if (!pmb)
 			goto error;
@@ -410,14 +346,11 @@ static int fio_pmemblk_queue(struct thread_data *td, struct io_u *io_u)
 	unsigned long long off;
 	unsigned long len;
 	void *buf;
-	int (*blkop) (PMEMblkpool *, void *, off_t) = (void *)pmemblk_write;
 
 	fio_ro_check(td, io_u);
 
 	switch (io_u->ddir) {
 	case DDIR_READ:
-		blkop = pmemblk_read;
-		/* fall through */
 	case DDIR_WRITE:
 		off = io_u->offset;
 		len = io_u->xfer_buflen;
@@ -435,7 +368,11 @@ static int fio_pmemblk_queue(struct thread_data *td, struct io_u *io_u)
 		off /= pmb->pmb_bsize;
 		len /= pmb->pmb_bsize;
 		while (0 < len) {
-			if (0 != blkop(pmb->pmb_pool, buf, off)) {
+			if (io_u->ddir == DDIR_READ &&
+			   0 != pmemblk_read(pmb->pmb_pool, buf, off)) {
+				io_u->error = errno;
+				break;
+			} else if (0 != pmemblk_write(pmb->pmb_pool, buf, off)) {
 				io_u->error = errno;
 				break;
 			}
-- 
2.9.3

[PATCH v2 2/4] pmemblk, dev-dax: Update descriptions

[Robert Elliott]

From: Robert Elliott <elliott@hpe.com>

Make the descriptions of the pmemblk and dev-dax ioengines
more symmetric.
---
 HOWTO | 10 ++++++----
 fio.1 |  6 ++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/HOWTO b/HOWTO
index 4354e46..9e1ffa6 100644
--- a/HOWTO
+++ b/HOWTO
@@ -903,11 +903,13 @@ ioengine=str	Defines how the job issues io to the file. The following
 				overwriting. The writetrim mode works well
 				for this constraint.
 
-			pmemblk	Read and write through the NVML libpmemblk
-				interface.
+			pmemblk	Read and write using filesystem DAX to a file
+				on a filesystem mounted with DAX on a persistent
+				memory device through the NVML libpmemblk library.
 
-			dev-dax Read and write through a DAX device exposed
-				from persistent memory.
+			dev-dax Read and write using device DAX to a persistent
+				memory device (e.g., /dev/dax0.0) through the
+				NVML libpmem library.
 
 			external Prefix to specify loading an external
 				IO engine object file. Append the engine
diff --git a/fio.1 b/fio.1
index f486276..56f2d11 100644
--- a/fio.1
+++ b/fio.1
@@ -811,10 +811,12 @@ and discarding before overwriting. The trimwrite mode works well for this
 constraint.
 .TP
 .B pmemblk
-Read and write through the NVML libpmemblk interface.
+Read and write using filesystem DAX to a file on a filesystem mounted with
+DAX on a persistent memory device through the NVML libpmemblk library.
 .TP
 .B dev-dax
-Read and write through a DAX device exposed from persistent memory.
+Read and write using device DAX to a persistent memory device
+(e.g., /dev/dax0.0) through the NVML libpmem library.
 .RE
 .P
 .RE
-- 
2.9.3

[PATCH v2 3/4] pmemblk, dev-dax: clean up error logs

[Robert Elliott]

Print the errno string rather than the number, and prefix
all log_err prints with pmemblk or dev-dax rather than fio.
---
 engines/dev-dax.c | 22 +++++++++++-----------
 engines/pmemblk.c |  9 ++++-----
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/engines/dev-dax.c b/engines/dev-dax.c
index 45aca4e..235a31e 100644
--- a/engines/dev-dax.c
+++ b/engines/dev-dax.c
@@ -106,7 +106,7 @@ static int fio_devdax_prep_limited(struct thread_data *td, struct io_u *io_u)
 	struct fio_devdax_data *fdd = FILE_ENG_DATA(f);
 
 	if (io_u->buflen > f->real_file_size) {
-		log_err("fio: bs too big for dev-dax engine\n");
+		log_err("dev-dax: bs too big for dev-dax engine\n");
 		return EIO;
 	}
 
@@ -213,8 +213,8 @@ static int fio_devdax_init(struct thread_data *td)
 
 	if ((o->rw_min_bs & page_mask) &&
 	    (o->fsync_blocks || o->fdatasync_blocks)) {
-		log_err("fio: mmap options dictate a minimum block size of "
-			"%llu bytes\n", (unsigned long long) page_size);
+		log_err("dev-dax: mmap options dictate a minimum block size of %llu bytes\n",
+			(unsigned long long) page_size);
 		return 1;
 	}
 
@@ -272,8 +272,8 @@ fio_devdax_get_file_size(struct thread_data *td, struct fio_file *f)
 
 	rc = stat(f->file_name, &st);
 	if (rc < 0) {
-		log_err("%s: failed to stat file %s: %d\n",
-			td->o.name, f->file_name, errno);
+		log_err("%s: failed to stat file %s (%s)\n",
+			td->o.name, f->file_name, strerror(errno));
 		return -errno;
 	}
 
@@ -282,8 +282,8 @@ fio_devdax_get_file_size(struct thread_data *td, struct fio_file *f)
 
 	rpath = realpath(spath, npath);
 	if (!rpath) {
-		log_err("%s: realpath on %s failed: %d\n",
-			td->o.name, spath, errno);
+		log_err("%s: realpath on %s failed (%s)\n",
+			td->o.name, spath, strerror(errno));
 		return -errno;
 	}
 
@@ -298,15 +298,15 @@ fio_devdax_get_file_size(struct thread_data *td, struct fio_file *f)
 
 	sfile = fopen(spath, "r");
 	if (!sfile) {
-		log_err("%s: fopen on %s failed: %d\n",
-			td->o.name, spath, errno);
+		log_err("%s: fopen on %s failed (%s)\n",
+			td->o.name, spath, strerror(errno));
 		return 1;
 	}
 
 	rc = fscanf(sfile, "%lu", &size);
 	if (rc < 0) {
-		log_err("%s: fscanf on %s failed: %d\n",
-			td->o.name, spath, errno);
+		log_err("%s: fscanf on %s failed (%s)\n",
+			td->o.name, spath, strerror(errno));
 		return 1;
 	}
 
diff --git a/engines/pmemblk.c b/engines/pmemblk.c
index 7b1affa..7c609cf 100644
--- a/engines/pmemblk.c
+++ b/engines/pmemblk.c
@@ -203,9 +203,8 @@ static fio_pmemblk_file_t pmb_open(const char *pathspec, int flags)
 			    pmemblk_create(path, bsize, fsize, 0644);
 		}
 		if (!pmb->pmb_pool) {
-			log_err
-			    ("fio: enable to open pmemblk pool file (errno %d)\n",
-			     errno);
+			log_err("pmemblk: unable to open pmemblk pool file %s (%s)\n",
+			     path, strerror(errno));
 			goto error;
 		}
 
@@ -267,14 +266,14 @@ static int pmb_get_flags(struct thread_data *td, uint64_t *pflags)
 	if (!td->o.use_thread) {
 		if (!thread_warned) {
 			thread_warned = 1;
-			log_err("fio: must set thread=1 for pmemblk engine\n");
+			log_err("pmemblk: must set thread=1 for pmemblk engine\n");
 		}
 		return 1;
 	}
 
 	if (!td->o.odirect && !odirect_warned) {
 		odirect_warned = 1;
-		log_info("fio: direct == 0, but pmemblk is always direct\n");
+		log_info("pmemblk: direct == 0, but pmemblk is always direct\n");
 	}
 
 	if (td->o.allow_create)
-- 
2.9.3

[PATCH v2 4/4] pmemblk: Clarify fsize is in MiB not MB

[Robert Elliott]

From: Robert Elliott <elliott@hpe.com>

Clarify that the FSIZE argument is in MiB (power of 2) rather
than MB (power of 10) in comments and variable names.
---
 engines/pmemblk.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/engines/pmemblk.c b/engines/pmemblk.c
index 7c609cf..c70015b 100644
--- a/engines/pmemblk.c
+++ b/engines/pmemblk.c
@@ -27,11 +27,11 @@
  *   ioengine=pmemblk
  *
  * Other relevant settings:
+ *   thread=1   REQUIRED
  *   iodepth=1
  *   direct=1
- *   thread=1   REQUIRED
  *   unlink=1
- *   filename=/pmem0/fiotestfile,BSIZE,FSIZEMB
+ *   filename=/mnt/pmem0/fiotestfile,BSIZE,FSIZEMiB
  *
  *   thread must be set to 1 for pmemblk as multiple processes cannot
  *     open the same block pool file.
@@ -39,14 +39,23 @@
  *   iodepth should be set to 1 as pmemblk is always synchronous.
  *   Use numjobs to scale up.
  *
- *   direct=1 is implied as pmemblk is always direct.
+ *   direct=1 is implied as pmemblk is always direct. A warning message
+ *   is printed if this is not specified.
+ *
+ *   unlink=1 removes the block pool file after testing, and is optional.
+ *
+ *   The pmem device must have a DAX-capable filesystem and be mounted
+ *   with DAX enabled.  filename must point to a file on that filesystem.
  *
- *   Can set unlink to 1 to remove the block pool file after testing.
+ *   Example:
+ *     mkfs.xfs /dev/pmem0
+ *     mkdir /mnt/pmem0
+ *     mount -o dax /dev/pmem0 /mnt/pmem0
  *
  *   When specifying the filename, if the block pool file does not already
- *   exist, then the pmemblk engine can create the pool file if you specify
+ *   exist, then the pmemblk engine creates the pool file if you specify
  *   the block and file sizes.  BSIZE is the block size in bytes.
- *   FSIZEMB is the pool file size in MB.
+ *   FSIZEMB is the pool file size in MiB.
  *
  *   See examples/pmemblk.fio for more.
  *
@@ -128,7 +137,7 @@ static void fio_pmemblk_cache_remove(fio_pmemblk_file_t pmb)
  * level, we allow the block size and file size to be appended
  * to the file name:
  *
- *   path[,bsize,fsizemb]
+ *   path[,bsize,fsizemib]
  *
  * note that we do not use the fio option "filesize" to dictate
  * the file size because we can only give libpmemblk the gross
@@ -138,7 +147,7 @@ static void fio_pmemblk_cache_remove(fio_pmemblk_file_t pmb)
  * the final path without the parameters is returned in ppath.
  * the block size and file size are returned in pbsize and fsize.
  *
- * note that the user should specify the file size in MiB, but
+ * note that the user specifies the file size in MiB, but
  * we return bytes from here.
  */
 static void pmb_parse_path(const char *pathspec, char **ppath, uint64_t *pbsize,
@@ -147,7 +156,7 @@ static void pmb_parse_path(const char *pathspec, char **ppath, uint64_t *pbsize,
 	char *path;
 	char *s;
 	uint64_t bsize;
-	uint64_t fsizemb;
+	uint64_t fsizemib;
 
 	path = strdup(pathspec);
 	if (!path) {
@@ -157,14 +166,14 @@ static void pmb_parse_path(const char *pathspec, char **ppath, uint64_t *pbsize,
 
 	/* extract sizes, if given */
 	s = strrchr(path, ',');
-	if (s && (fsizemb = strtoull(s + 1, NULL, 10))) {
+	if (s && (fsizemib = strtoull(s + 1, NULL, 10))) {
 		*s = 0;
 		s = strrchr(path, ',');
 		if (s && (bsize = strtoull(s + 1, NULL, 10))) {
 			*s = 0;
 			*ppath = path;
 			*pbsize = bsize;
-			*pfsize = fsizemb << 20;
+			*pfsize = fsizemib << 20;
 			return;
 		}
 	}
-- 
2.9.3

Re: [PATCH v2 1/4] pmemblk, dev-dax: load libpmem and libpmemblk at startup

[Jens Axboe]

On Tue, Jan 10 2017, Robert Elliott wrote:
> From: Robert Elliott <elliott@hpe.com>
> 
> The pmemblk and dev-dax ioengines were loading libpmem.so and
> libpmemblk.so using dlopen() at runtime.
> 
> Although the upstream nvml Makefile installs a libpmem.so
> symbolic link, some of the distros (e.g. SUSE Linux Enterprise
> Server 12 SP2) are just installing so.1 symbolic links.  So,
> fio fails to find the libpmem.so and libpmemblk.so library files.
> 
> http://www.ibm.com/developerworks/linux/library/l-shlibs/index.html
> says applications should always load the versioned filenames to
> avoid compatibility issues; the non-versioned filenames are just
> for the compiler and linker.
> 
> Change ./configure to pass -lpmem and -lpmemblk to the compiler
> so the fio binary loads the versioned filename at startup like
> the other libraries, rather than open the non-versioned filename
> with dlopen() during runtime.
> 
> This way they show up in ldd:
> $ ldd fio
>         linux-vdso.so.1 (0x00007ffc290aa000)
>         libpmemblk.so.1 => /lib64/libpmemblk.so.1 (0x00007f65b5bc4000)
>         libpmem.so.1 => /lib64/libpmem.so.1 (0x00007f65b59be000)
>         libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f65b57b3000)
>         librt.so.1 => /lib64/librt.so.1 (0x00007f65b55ab000)
>         libaio.so.1 => /lib64/libaio.so.1 (0x00007f65b53a9000)
>         libz.so.1 => /lib64/libz.so.1 (0x00007f65b5191000)
>         libm.so.6 => /lib64/libm.so.6 (0x00007f65b4e88000)
>         libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f65b4c6a000)
>         libdl.so.2 => /lib64/libdl.so.2 (0x00007f65b4a66000)
>         libc.so.6 => /lib64/libc.so.6 (0x00007f65b46a0000)
>         libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f65b449b000)
>         /lib64/ld-linux-x86-64.so.2 (0x0000559f4d872000)

Added 1-4, thanks Robert.

-- 
Jens Axboe