[dm-devel] [PATCH 5/6] libmultipath: simplify failed wwid code

2020-05-14 Thread Benjamin Marzinski 
The (is|mark|unmark)_failed_wwid code is needlessly complicated.
Locking a file is necssary if multiple processes could otherwise be
writing to it at the same time. That is not the case with the
failed_wwids files. They can simply be empty files in a directory.  Even
with all the locking in place, two processes accessing or modifying a
file at the same time will still race. And even without the locking, if
two processes try to access or modify a file at the same time, they will
both see a reasonable result, and will leave the files in a valid state
afterwards.

Instead of doing all the locking work (which made it necessary to write
a file, even just to check if a file existed), simply check for files
with lstat(), create them with open(), and remove them with unlink().

Signed-off-by: Benjamin Marzinski 
---
 libmultipath/wwids.c | 131 ++-
 1 file changed, 56 insertions(+), 75 deletions(-)

diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index 637cb0ab..aab5da8a 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "util.h"
 #include "checkers.h"
@@ -348,109 +349,89 @@ remember_wwid(char *wwid)
 }
 
 static const char shm_dir[] = MULTIPATH_SHM_BASE "failed_wwids";
-static const char shm_lock[] = ".lock";
-static const char shm_header[] = "multipath shm lock file, don't edit";
-static char _shm_lock_path[sizeof(shm_dir)+sizeof(shm_lock)];
-static const char *shm_lock_path = &_shm_lock_path[0];
 
-static void init_shm_paths(void)
+static void print_failed_wwid_result(const char * msg, const char *wwid, int r)
 {
-   snprintf(_shm_lock_path, sizeof(_shm_lock_path),
-"%s/%s", shm_dir, shm_lock);
+   switch(r) {
+   case WWID_FAILED_ERROR:
+   condlog(1, "%s: %s: %m", msg, wwid);
+   return;
+   case WWID_IS_FAILED:
+   case WWID_IS_NOT_FAILED:
+   condlog(4, "%s: %s is %s", msg, wwid,
+   r == WWID_IS_FAILED ? "failed" : "good");
+   return;
+   case WWID_FAILED_CHANGED:
+   condlog(3, "%s: %s", msg, wwid);
+   }
 }
 
-static pthread_once_t shm_path_once = PTHREAD_ONCE_INIT;
-
-static int multipath_shm_open(bool rw)
+int is_failed_wwid(const char *wwid)
 {
-   int fd;
-   int can_write;
-
-   pthread_once(_path_once, init_shm_paths);
-   fd = open_file(shm_lock_path, _write, shm_header);
+   struct stat st;
+   char path[PATH_MAX];
+   int r;
 
-   if (fd >= 0 && rw && !can_write) {
-   close(fd);
-   condlog(1, "failed to open %s for writing", shm_dir);
+   if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
+   condlog(1, "%s: path name overflow", __func__);
return -1;
}
 
-   return fd;
-}
-
-static void multipath_shm_close(void *arg)
-{
-   long fd = (long)arg;
+   if (lstat(path, ) == 0)
+   r = WWID_IS_FAILED;
+   else if (errno == ENOENT)
+   r = WWID_IS_NOT_FAILED;
+   else
+   r = WWID_FAILED_ERROR;
 
-   close(fd);
-   unlink(shm_lock_path);
+   print_failed_wwid_result("is_failed", wwid, r);
+   return r;
 }
 
-static int _failed_wwid_op(const char *wwid, bool rw,
-  int (*func)(const char *), const char *msg)
+int mark_failed_wwid(const char *wwid)
 {
char path[PATH_MAX];
-   long lockfd;
-   int r = -1;
+   int r, fd;
 
if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
condlog(1, "%s: path name overflow", __func__);
return -1;
}
-
-   lockfd = multipath_shm_open(rw);
-   if (lockfd == -1)
+   if (ensure_directories_exist(path, 0700) < 0) {
+   condlog(1, "%s: can't setup directories", __func__);
return -1;
+   }
 
-   pthread_cleanup_push(multipath_shm_close, (void *)lockfd);
-   r = func(path);
-   pthread_cleanup_pop(1);
-
-   if (r == WWID_FAILED_ERROR)
-   condlog(1, "%s: %s: %s", msg, wwid, strerror(errno));
-   else if (r == WWID_FAILED_CHANGED)
-   condlog(3, "%s: %s", msg, wwid);
-   else if (!rw)
-   condlog(4, "%s: %s is %s", msg, wwid,
-   r == WWID_IS_FAILED ? "failed" : "good");
+   fd = open(path, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR);
+   if (fd >= 0) {
+   close(fd);
+   r = WWID_FAILED_CHANGED;
+   } else if (errno == EEXIST)
+   r = WWID_FAILED_UNCHANGED;
+   else
+   r = WWID_FAILED_ERROR;
 
+   print_failed_wwid_result("mark_failed", wwid, r);
return r;
 }
 
-static int _is_failed(const char *path)
+int unmark_failed_wwid(const char *wwid)
 {
-   struct stat st;
+   char path[PATH_MAX];
+   int r;
 
-   if (lstat(path, ) == 0)
-   return WWID_IS_FAILED;

[dm-devel] [PATCH 6/6] libmultipath: use atomic linkat() in mark_failed_wwid()

2020-05-14 Thread Benjamin Marzinski 
From: Martin Wilck 

This keeps (almost) the simplicity of the previous patch, while
making sure that the return value of mark_failed_wwid()
(WWID_FAILED_CHANGED vs. WWID_FAILED_UNCHANGED) is correct, even
if several processes access this WWID at the same time.

Signed-off-by: Martin Wilck 
Signed-off-by: Benjamin Marzinski 
---
 libmultipath/wwids.c | 42 +-
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index aab5da8a..61d9c39e 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -374,7 +374,7 @@ int is_failed_wwid(const char *wwid)
 
if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
condlog(1, "%s: path name overflow", __func__);
-   return -1;
+   return WWID_FAILED_ERROR;
}
 
if (lstat(path, ) == 0)
@@ -390,27 +390,43 @@ int is_failed_wwid(const char *wwid)
 
 int mark_failed_wwid(const char *wwid)
 {
-   char path[PATH_MAX];
-   int r, fd;
+   char tmpfile[WWID_SIZE + 2 * sizeof(long) + 1];
+   int r = WWID_FAILED_ERROR, fd, dfd;
 
-   if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
-   condlog(1, "%s: path name overflow", __func__);
-   return -1;
+   dfd = open(shm_dir, O_RDONLY|O_DIRECTORY);
+   if (dfd == -1 && errno == ENOENT) {
+   char path[sizeof(shm_dir) + 2];
+
+   /* arg for ensure_directories_exist() must not end with "/" */
+   safe_sprintf(path, "%s/_", shm_dir);
+   ensure_directories_exist(path, 0700);
+   dfd = open(shm_dir, O_RDONLY|O_DIRECTORY);
}
-   if (ensure_directories_exist(path, 0700) < 0) {
-   condlog(1, "%s: can't setup directories", __func__);
-   return -1;
+   if (dfd == -1) {
+   condlog(1, "%s: can't setup %s: %m", __func__, shm_dir);
+   return WWID_FAILED_ERROR;
}
 
-   fd = open(path, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR);
-   if (fd >= 0) {
+   safe_sprintf(tmpfile, "%s.%lx", wwid, (long)getpid());
+   fd = openat(dfd, tmpfile, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR);
+   if (fd >= 0)
close(fd);
+   else
+   goto out_closedir;
+
+   if (linkat(dfd, tmpfile, dfd, wwid, 0) == 0)
r = WWID_FAILED_CHANGED;
-   } else if (errno == EEXIST)
+   else if (errno == EEXIST)
r = WWID_FAILED_UNCHANGED;
else
r = WWID_FAILED_ERROR;
 
+   if (unlinkat(dfd, tmpfile, 0) == -1)
+   condlog(2, "%s: failed to unlink %s/%s: %m",
+   __func__, shm_dir, tmpfile);
+
+out_closedir:
+   close(dfd);
print_failed_wwid_result("mark_failed", wwid, r);
return r;
 }
@@ -422,7 +438,7 @@ int unmark_failed_wwid(const char *wwid)
 
if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
condlog(1, "%s: path name overflow", __func__);
-   return -1;
+   return WWID_FAILED_ERROR;
}
 
if (unlink(path) == 0)
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 2/6] libmultipath: make sysfs_is_multipathed able to return wwid

2020-05-14 Thread Benjamin Marzinski 
sysfs_is_multipathed reads the wwid of the dm device holding a path to
check if its a multipath device.  Add code to optinally set pp->wwid to
that wwid.  This will be used by a future patch.

Signed-off-by: Benjamin Marzinski 
---
 libmultipath/sysfs.c | 24 +++-
 libmultipath/sysfs.h |  2 +-
 multipath/main.c |  7 ---
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index 62ec2ed7..12a82d95 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -295,7 +295,7 @@ static int select_dm_devs(const struct dirent *di)
return fnmatch("dm-*", di->d_name, FNM_FILE_NAME) == 0;
 }
 
-bool sysfs_is_multipathed(const struct path *pp)
+bool sysfs_is_multipathed(struct path *pp, bool set_wwid)
 {
char pathbuf[PATH_MAX];
struct scandir_result sr;
@@ -325,7 +325,7 @@ bool sysfs_is_multipathed(const struct path *pp)
for (i = 0; i < r && !found; i++) {
long fd;
int nr;
-   char uuid[6];
+   char uuid[WWID_SIZE + UUID_PREFIX_LEN];
 
if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n,
  "/%s/dm/uuid", di[i]->d_name))
@@ -339,12 +339,26 @@ bool sysfs_is_multipathed(const struct path *pp)
 
pthread_cleanup_push(close_fd, (void *)fd);
nr = read(fd, uuid, sizeof(uuid));
-   if (nr == sizeof(uuid) && !memcmp(uuid, "mpath-", sizeof(uuid)))
+   if (nr > (int)UUID_PREFIX_LEN &&
+   !memcmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN))
found = true;
else if (nr < 0) {
-   condlog(1, "%s: error reading from %s: %s",
-   __func__, pathbuf, strerror(errno));
+   condlog(1, "%s: error reading from %s: %m",
+   __func__, pathbuf);
}
+   if (found && set_wwid) {
+   nr -= UUID_PREFIX_LEN;
+   memcpy(pp->wwid, uuid + UUID_PREFIX_LEN, nr);
+   if (nr == WWID_SIZE) {
+   condlog(4, "%s: overflow while reading from %s",
+   __func__, pathbuf);
+   pp->wwid[0] = '\0';
+   } else {
+   pp->wwid[nr] = '\0';
+   strchop(pp->wwid);
+   }
+}
+
pthread_cleanup_pop(1);
}
pthread_cleanup_pop(1);
diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h
index 9ae30b39..72b39ab2 100644
--- a/libmultipath/sysfs.h
+++ b/libmultipath/sysfs.h
@@ -14,5 +14,5 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, 
const char *attr_name,
 unsigned char * value, size_t value_len);
 int sysfs_get_size (struct path *pp, unsigned long long * size);
 int sysfs_check_holders(char * check_devt, char * new_devt);
-bool sysfs_is_multipathed(const struct path *pp);
+bool sysfs_is_multipathed(struct path *pp, bool set_wwid);
 #endif
diff --git a/multipath/main.c b/multipath/main.c
index cf9d2a28..545ead87 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -638,7 +638,8 @@ configure (struct config *conf, enum mpath_cmds cmd,
 * Shortcut for find_multipaths smart:
 * Quick check if path is already multipathed.
 */
-   if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0))) {
+   if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0),
+false)) {
r = RTVL_YES;
goto print_valid;
}
@@ -747,8 +748,8 @@ configure (struct config *conf, enum mpath_cmds cmd,
/*
 * Check if we raced with multipathd
 */
-   r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0)) ?
-   RTVL_YES : RTVL_NO;
+   r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0),
+false) ? RTVL_YES : RTVL_NO;
}
goto print_valid;
}
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 3/6] multipath: centralize validation code

2020-05-14 Thread Benjamin Marzinski 
This code pulls the multipath path validation code out of configure(),
and puts it into its own function, check_path_valid(). This function
calls a new libmultipath function, is_path_valid() to check just path
requested. This seperation exists so that is_path_valid() can be reused
by future code. This code will give almost the same answer as the
existing code, with the exception that now, if a device is currently
multipathed, it will always be a valid multipath path.

Signed-off-by: Benjamin Marzinski 
---
 libmultipath/Makefile|   2 +-
 libmultipath/devmapper.c |  49 +++
 libmultipath/devmapper.h |   1 +
 libmultipath/structs.h   |  24 +---
 libmultipath/valid.c | 118 
 libmultipath/valid.h |  32 +
 libmultipath/wwids.c |  10 +-
 multipath/main.c | 296 +--
 8 files changed, 337 insertions(+), 195 deletions(-)
 create mode 100644 libmultipath/valid.c
 create mode 100644 libmultipath/valid.h

diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index f19b7ad2..e5dac5ea 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -48,7 +48,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \
log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \
lock.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
io_err_stat.o dm-generic.o generic.o foreign.o nvme-lib.o \
-   libsg.o
+   libsg.o valid.o
 
 all: $(LIBS)
 
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 7ed494a1..92f61133 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -770,6 +770,55 @@ out:
return r;
 }
 
+/*
+ * Return
+ *   1 : map with uuid exists
+ *   0 : map with uuid doesn't exist
+ *  -1 : error
+ */
+int
+dm_map_present_by_uuid(const char *uuid)
+{
+   struct dm_task *dmt;
+   struct dm_info info;
+   char prefixed_uuid[WWID_SIZE + UUID_PREFIX_LEN];
+   int r = -1;
+
+   if (!uuid || uuid[0] == '\0')
+   return 0;
+
+   if (safe_sprintf(prefixed_uuid, UUID_PREFIX "%s", uuid))
+   goto out;
+
+   if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
+   goto out;
+
+   dm_task_no_open_count(dmt);
+
+   if (!dm_task_set_uuid(dmt, prefixed_uuid))
+   goto out_task;
+
+   if (!dm_task_run(dmt))
+   goto out_task;
+
+   if (!dm_task_get_info(dmt, ))
+   goto out_task;
+
+   r = 0;
+
+   if (!info.exists)
+   goto out_task;
+
+   r = 1;
+out_task:
+   dm_task_destroy(dmt);
+out:
+   if (r < 0)
+   condlog(3, "%s: dm command failed in %s: %s", uuid,
+   __FUNCTION__, strerror(errno));
+   return r;
+}
+
 static int
 dm_dev_t (const char * mapname, char * dev_t, int len)
 {
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 17fc9faf..5ed7edc5 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -39,6 +39,7 @@ int dm_simplecmd_noflush (int, const char *, uint16_t);
 int dm_addmap_create (struct multipath *mpp, char *params);
 int dm_addmap_reload (struct multipath *mpp, char *params, int flush);
 int dm_map_present (const char *);
+int dm_map_present_by_uuid(const char *uuid);
 int dm_get_map(const char *, unsigned long long *, char *);
 int dm_get_status(const char *, char *);
 int dm_type(const char *, char *);
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 9bd39eb1..d69bc2e9 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -101,29 +101,13 @@ enum yes_no_undef_states {
YNU_YES,
 };
 
-#define _FIND_MULTIPATHS_F (1 << 1)
-#define _FIND_MULTIPATHS_I (1 << 2)
-#define _FIND_MULTIPATHS_N (1 << 3)
-/*
- * _FIND_MULTIPATHS_F must have the same value as YNU_YES.
- * Generate a compile time error if that isn't the case.
- */
-extern char ___error1___[-(_FIND_MULTIPATHS_F != YNU_YES)];
-
-#define find_multipaths_on(conf) \
-   (!!((conf)->find_multipaths & _FIND_MULTIPATHS_F))
-#define ignore_wwids_on(conf) \
-   (!!((conf)->find_multipaths & _FIND_MULTIPATHS_I))
-#define ignore_new_devs_on(conf) \
-   (!!((conf)->find_multipaths & _FIND_MULTIPATHS_N))
-
 enum find_multipaths_states {
FIND_MULTIPATHS_UNDEF = YNU_UNDEF,
FIND_MULTIPATHS_OFF = YNU_NO,
-   FIND_MULTIPATHS_ON = _FIND_MULTIPATHS_F,
-   FIND_MULTIPATHS_GREEDY = _FIND_MULTIPATHS_I,
-   FIND_MULTIPATHS_SMART = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_I,
-   FIND_MULTIPATHS_STRICT = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_N,
+   FIND_MULTIPATHS_ON = YNU_YES,
+   FIND_MULTIPATHS_GREEDY,
+   FIND_MULTIPATHS_SMART,
+   FIND_MULTIPATHS_STRICT,
__FIND_MULTIPATHS_LAST,
 };
 
diff --git a/libmultipath/valid.c b/libmultipath/valid.c
new file mode 100644
index ..456b1f6e
--- /dev/null
+++ b/libmultipath/valid.c
@@ -0,0 +1,118 @@
+/*
+  Copyright (c) 2020 Benjamin Marzinski, IBM
+
+  This program

[dm-devel] [PATCH 1/6] libmultipath: make libmp_dm_init optional

2020-05-14 Thread Benjamin Marzinski 
Move dm_initialized out of libmp_dm_task_create(), and add
a function skip_libmp_dm_init() so that users of libmultipath can skip
initializing device-mapper. This is needed for other programs that
use libmultipath (or a library that depends on it) but want to control
how device-mapper is set up.

Also make dm_prereq a global function.

Reviewed-by: Martin Wilck 
Signed-off-by: Benjamin Marzinski 
---
 libmultipath/devmapper.c | 17 +
 libmultipath/devmapper.h |  3 ++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 13a1cf53..7ed494a1 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -33,6 +33,8 @@
 #define MAX_WAIT 5
 #define LOOPS_PER_SEC 5
 
+static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT;
+
 static int dm_conf_verbosity;
 
 #ifdef LIBDM_API_DEFERRED
@@ -229,7 +231,7 @@ dm_tgt_prereq (unsigned int *ver)
return 1;
 }
 
-static int dm_prereq(unsigned int *v)
+int dm_prereq(unsigned int *v)
 {
if (dm_lib_prereq())
return 1;
@@ -243,7 +245,7 @@ void libmp_udev_set_sync_support(int on)
libmp_dm_udev_sync = !!on;
 }
 
-void libmp_dm_init(void)
+static void libmp_dm_init(void)
 {
struct config *conf;
int verbosity;
@@ -262,11 +264,18 @@ void libmp_dm_init(void)
dm_udev_set_sync_support(libmp_dm_udev_sync);
 }
 
+static void _do_skip_libmp_dm_init(void)
+{
+}
+
+void skip_libmp_dm_init(void)
+{
+   pthread_once(_initialized, _do_skip_libmp_dm_init);
+}
+
 struct dm_task*
 libmp_dm_task_create(int task)
 {
-   static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT;
-
pthread_once(_initialized, libmp_dm_init);
return dm_task_create(task);
 }
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 7557a86b..17fc9faf 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -28,7 +28,8 @@
 #define UUID_PREFIX_LEN (sizeof(UUID_PREFIX) - 1)
 
 void dm_init(int verbosity);
-void libmp_dm_init(void);
+int dm_prereq(unsigned int *v);
+void skip_libmp_dm_init(void);
 void libmp_udev_set_sync_support(int on);
 struct dm_task *libmp_dm_task_create(int task);
 int dm_drv_version (unsigned int * version);
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 4/6] Unit tests for is_path_valid()

2020-05-14 Thread Benjamin Marzinski 
Signed-off-by: Benjamin Marzinski 
---
 tests/Makefile |   4 +-
 tests/valid.c  | 424 +
 2 files changed, 427 insertions(+), 1 deletion(-)
 create mode 100644 tests/valid.c

diff --git a/tests/Makefile b/tests/Makefile
index 1b8706a7..125553b8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -13,7 +13,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir) \
 LIBDEPS += -L$(multipathdir) -L$(mpathcmddir) -lmultipath -lmpathcmd -lcmocka
 
 TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \
-alias directio
+alias directio valid
 
 .SILENT: $(TESTS:%=%.o)
 .PRECIOUS: $(TESTS:%=%-test)
@@ -50,6 +50,8 @@ vpd-test_OBJDEPS :=  ../libmultipath/discovery.o
 vpd-test_LIBDEPS := -ludev -lpthread -ldl
 alias-test_TESTDEPS := test-log.o
 alias-test_LIBDEPS := -lpthread -ldl
+valid-test_OBJDEPS := ../libmultipath/valid.o
+valid-test_LIBDEPS := -ludev -lpthread -ldl
 ifneq ($(DIO_TEST_DEV),)
 directio-test_LIBDEPS := -laio
 endif
diff --git a/tests/valid.c b/tests/valid.c
new file mode 100644
index ..b128b029
--- /dev/null
+++ b/tests/valid.c
@@ -0,0 +1,424 @@
+/*
+ * Copyright (c) 2020 Benjamin Marzinski, Redhat
+ *
+ * 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; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will 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, see .
+ *
+ */
+
+#define _GNU_SOURCE
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "globals.c"
+#include "util.h"
+#include "discovery.h"
+#include "wwids.h"
+#include "valid.h"
+
+int test_fd;
+struct udev_device {
+   int unused;
+} test_udev;
+
+bool __wrap_sysfs_is_multipathed(struct path *pp, bool set_wwid)
+{
+   bool is_multipathed = mock_type(bool);
+   assert_non_null(pp);
+   assert_int_not_equal(strlen(pp->dev), 0);
+   if (is_multipathed && set_wwid)
+   strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE);
+   return is_multipathed;
+}
+
+int __wrap___mpath_connect(int nonblocking)
+{
+   bool connected = mock_type(bool);
+   assert_int_equal(nonblocking, 1);
+   if (connected)
+   return test_fd;
+   errno = mock_type(int);
+   return -1;
+}
+
+int __wrap_systemd_service_enabled(const char *dev)
+{
+   return (int)mock_type(bool);
+}
+
+/* There's no point in checking the return value here */
+int __wrap_mpath_disconnect(int fd)
+{
+   assert_int_equal(fd, test_fd);
+   return 0;
+}
+
+struct udev_device *__wrap_udev_device_new_from_subsystem_sysname(struct udev 
*udev, const char *subsystem, const char *sysname)
+{
+   bool passed = mock_type(bool);
+   assert_string_equal(sysname, mock_ptr_type(char *));
+   if (passed)
+   return _udev;
+   return NULL;
+}
+
+int __wrap_pathinfo(struct path *pp, struct config *conf, int mask)
+{
+   int ret = mock_type(int);
+   assert_string_equal(pp->dev, mock_ptr_type(char *));
+   assert_int_equal(mask, DI_SYSFS | DI_WWID | DI_BLACKLIST);
+   if (ret == PATHINFO_OK)
+   strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE);
+   else
+   memset(pp->wwid, 0, WWID_SIZE);
+   return ret;
+}
+
+int __wrap_is_failed_wwid(const char *wwid)
+{
+   int ret = mock_type(int);
+   assert_string_equal(wwid, mock_ptr_type(char *));
+   return ret;
+}
+
+int __wrap_check_wwids_file(char *wwid, int write_wwid)
+{
+   bool passed = mock_type(bool);
+   assert_int_equal(write_wwid, 0);
+   assert_string_equal(wwid, mock_ptr_type(char *));
+   if (passed)
+   return 0;
+   else
+   return -1;
+}
+
+int __wrap_dm_map_present_by_uuid(const char *uuid)
+{
+   int ret = mock_type(int);
+   assert_string_equal(uuid, mock_ptr_type(char *));
+   return ret;
+}
+
+enum {
+   STAGE_IS_MULTIPATHED,
+   STAGE_CHECK_MULTIPATHD,
+   STAGE_GET_UDEV_DEVICE,
+   STAGE_PATHINFO,
+   STAGE_IS_FAILED,
+   STAGE_CHECK_WWIDS,
+   STAGE_UUID_PRESENT,
+};
+
+/* setup the test to continue past the given stage in is_path_valid() */
+static void setup_passing(char *name, char *wwid, bool check_multipathd,
+ unsigned int stage)
+{
+   will_return(__wrap_sysfs_is_multipathed, false);
+   if (stage == STAGE_IS_MULTIPATHED)
+   return;
+   if (check_multipathd)
+

Re: [dm-devel] [PATCH 0/2] More minor libmultipath fixes

2020-05-14 Thread Benjamin Marzinski 
On Tue, May 12, 2020 at 10:38:20PM +0200, mwi...@suse.com wrote:
> From: Martin Wilck 
> 
> Hi Christophe, hi Ben,
> 
> this is a follow-up on yesterday's "Minor fixes for multipath-tools"
> series. It adds fixes for sporadic build failures I observed.
> The libsg move is a long-overdue cleanup.
> 

Reviewed-by: Benjamin Marzinski 

For the set

> Regards
> Martin
> 
> Martin Wilck (2):
>   libmultipath: move libsg into libmultipath
>   multipath-tools Makefile: add install dependency
> 
>  Makefile| 4 
>  libmultipath/Makefile   | 3 ++-
>  libmultipath/checkers/Makefile  | 6 +++---
>  libmultipath/{checkers => }/libsg.c | 0
>  libmultipath/{checkers => }/libsg.h | 0
>  libmultipath/prioritizers/Makefile  | 2 +-
>  6 files changed, 10 insertions(+), 5 deletions(-)
>  rename libmultipath/{checkers => }/libsg.c (100%)
>  rename libmultipath/{checkers => }/libsg.h (100%)
> 
> -- 
> 2.26.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 00/11] Minor fixes for multipath-tools

2020-05-14 Thread Benjamin Marzinski 
On Tue, May 12, 2020 at 12:39:20AM +0200, mwi...@suse.com wrote:
> From: Martin Wilck 
> 
> Hi Christophe, hi Ben,
> 
> This series fixes a couple of issues I recently found with the 32bit build
> of multipath-tools, and after integrating our CI into the automated builds
> on our build server.
> 
> Regards
> Martin

Reviewed-by: Benjamin Marzinski 

For the set.

> 
> Martin Wilck (11):
>   multipath-tools: Makefile: more dependency fixes for parallel build
>   multipath-tools: Makefile.inc: separate out OPTFLAGS
>   multipath-tools: Makefile.inc: allow user settings for LDFLAGS
>   multipath-tools: Makefile.inc: set -Wno-error=clobbered
>   libmultipath: discovery.c: use %z qualifier for size_t
>   libmultipath: eliminate more signed/unsigned comparisons
>   libmultipath: set_uint: fix parsing for 32bit
>   multipath-tools tests/Makefile: add -lmpathcmd to LIBDEPS
>   multipath tools tests/Makefile: Fix OBJDEPS for hwtable-test
>   multipath-tools tests/test-lib.c: drop __wrap_is_claimed_by_foreign
>   multipath-tools tests/directio: fix -Wmaybe-uninitalized warning
> 
>  Makefile  |  5 +++--
>  Makefile.inc  | 16 
>  libmpathpersist/mpath_pr_ioctl.c  |  2 +-
>  libmultipath/dict.c   | 11 +++
>  libmultipath/discovery.c  | 16 
>  libmultipath/print.c  | 12 ++--
>  libmultipath/prioritizers/alua_spc3.h |  2 +-
>  multipathd/cli_handlers.c | 20 ++--
>  multipathd/main.c |  2 +-
>  tests/Makefile|  4 ++--
>  tests/directio.c  |  2 +-
>  tests/test-lib.c  |  6 --
>  12 files changed, 48 insertions(+), 50 deletions(-)
> 
> -- 
> 2.26.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [dm:dm-5.8 55/57] include/linux/device-mapper.h:626:2: error: #endif without #if

2020-05-14 Thread Mike Snitzer 
Sorry about that, now fixed.

On Thu, May 14 2020 at  5:15pm -0400,
kbuild test robot  wrote:

> tree:   
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git 
> dm-5.8
> head:   2e374fada9514fe10561a41838a52dc05f0317b4
> commit: 6e1746c86200d4cd2562abe0a4b2892e3be04271 [55/57] dm: use dynamic 
> debug instead of compile-time config option
> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gcc (GCC) 9.3.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout 6e1746c86200d4cd2562abe0a4b2892e3be04271
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=sh 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot 
> 
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-uevent.c:14:
> >> include/linux/device-mapper.h:626:2: error: #endif without #if
> 626 | #endif /* _LINUX_DEVICE_MAPPER_H */
> |  ^
> --
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-linear.c:7:
> >> include/linux/device-mapper.h:626:2: error: #endif without #if
> 626 | #endif /* _LINUX_DEVICE_MAPPER_H */
> |  ^
> In file included from drivers/md/dm-linear.c:14:
> >> include/linux/device-mapper.h:616:24: error: redefinition of 'to_sector'
> 616 | static inline sector_t to_sector(unsigned long long n)
> |^
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-linear.c:7:
> include/linux/device-mapper.h:616:24: note: previous definition of 
> 'to_sector' was here
> 616 | static inline sector_t to_sector(unsigned long long n)
> |^
> In file included from drivers/md/dm-linear.c:14:
> >> include/linux/device-mapper.h:621:29: error: redefinition of 'to_bytes'
> 621 | static inline unsigned long to_bytes(sector_t n)
> | ^~~~
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-linear.c:7:
> include/linux/device-mapper.h:621:29: note: previous definition of 'to_bytes' 
> was here
> 621 | static inline unsigned long to_bytes(sector_t n)
> | ^~~~
> In file included from drivers/md/dm-linear.c:14:
> >> include/linux/device-mapper.h:626:2: error: #endif without #if
> 626 | #endif /* _LINUX_DEVICE_MAPPER_H */
> |  ^
> --
> In file included from drivers/md/dm-snap.c:8:
> >> include/linux/device-mapper.h:626:2: error: #endif without #if
> 626 | #endif /* _LINUX_DEVICE_MAPPER_H */
> |  ^
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-snap.c:22:
> >> include/linux/device-mapper.h:616:24: error: redefinition of 'to_sector'
> 616 | static inline sector_t to_sector(unsigned long long n)
> |^
> In file included from drivers/md/dm-snap.c:8:
> include/linux/device-mapper.h:616:24: note: previous definition of 
> 'to_sector' was here
> 616 | static inline sector_t to_sector(unsigned long long n)
> |^
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-snap.c:22:
> >> include/linux/device-mapper.h:621:29: error: redefinition of 'to_bytes'
> 621 | static inline unsigned long to_bytes(sector_t n)
> | ^~~~
> In file included from drivers/md/dm-snap.c:8:
> include/linux/device-mapper.h:621:29: note: previous definition of 'to_bytes' 
> was here
> 621 | static inline unsigned long to_bytes(sector_t n)
> | ^~~~
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-snap.c:22:
> >> include/linux/device-mapper.h:626:2: error: #endif without #if
> 626 | #endif /* _LINUX_DEVICE_MAPPER_H */
> |  ^
> In file included from drivers/md/dm-exception-store.h:15,
> from drivers/md/dm-snap.c:24:
> >> include/linux/device-mapper.h:616:24: error: redefinition of 'to_sector'
> 616 | static inline sector_t to_sector(unsigned long long n)
> |^
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-snap.c:22:
> include/linux/device-mapper.h:616:24: note: previous definition of 
> 'to_sector' was here
> 616 | static inline sector_t to_sector(unsigned long long n)
> |^
> In file included from drivers/md/dm-exception-store.h:15,
> from drivers/md/dm-snap.c:24:
> >> include/linux/device-mapper.h:621:29: error: redefinition of 'to_bytes'
> 621 | static inline unsigned long to_bytes(sector_t n)
> | ^~~~
> In file included from drivers/md/dm.h:14,
> from drivers/md/dm-snap.c:22:
> include/linux/device-mapper.h:621:29: note: previous definition of 'to_bytes' 
> was here
> 621 | static inline unsigned long to_bytes(sector_t n)
> | ^~~~
> In

[dm-devel] [dm:dm-5.8 55/57] include/linux/device-mapper.h:626:2: error: #endif without #if

2020-05-14 Thread kbuild test robot 
tree:   
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git 
dm-5.8
head:   2e374fada9514fe10561a41838a52dc05f0317b4
commit: 6e1746c86200d4cd2562abe0a4b2892e3be04271 [55/57] dm: use dynamic debug 
instead of compile-time config option
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 6e1746c86200d4cd2562abe0a4b2892e3be04271
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot 

All errors (new ones prefixed by >>, old ones prefixed by <<):

In file included from drivers/md/dm.h:14,
from drivers/md/dm-uevent.c:14:
>> include/linux/device-mapper.h:626:2: error: #endif without #if
626 | #endif /* _LINUX_DEVICE_MAPPER_H */
|  ^
--
In file included from drivers/md/dm.h:14,
from drivers/md/dm-linear.c:7:
>> include/linux/device-mapper.h:626:2: error: #endif without #if
626 | #endif /* _LINUX_DEVICE_MAPPER_H */
|  ^
In file included from drivers/md/dm-linear.c:14:
>> include/linux/device-mapper.h:616:24: error: redefinition of 'to_sector'
616 | static inline sector_t to_sector(unsigned long long n)
|^
In file included from drivers/md/dm.h:14,
from drivers/md/dm-linear.c:7:
include/linux/device-mapper.h:616:24: note: previous definition of 'to_sector' 
was here
616 | static inline sector_t to_sector(unsigned long long n)
|^
In file included from drivers/md/dm-linear.c:14:
>> include/linux/device-mapper.h:621:29: error: redefinition of 'to_bytes'
621 | static inline unsigned long to_bytes(sector_t n)
| ^~~~
In file included from drivers/md/dm.h:14,
from drivers/md/dm-linear.c:7:
include/linux/device-mapper.h:621:29: note: previous definition of 'to_bytes' 
was here
621 | static inline unsigned long to_bytes(sector_t n)
| ^~~~
In file included from drivers/md/dm-linear.c:14:
>> include/linux/device-mapper.h:626:2: error: #endif without #if
626 | #endif /* _LINUX_DEVICE_MAPPER_H */
|  ^
--
In file included from drivers/md/dm-snap.c:8:
>> include/linux/device-mapper.h:626:2: error: #endif without #if
626 | #endif /* _LINUX_DEVICE_MAPPER_H */
|  ^
In file included from drivers/md/dm.h:14,
from drivers/md/dm-snap.c:22:
>> include/linux/device-mapper.h:616:24: error: redefinition of 'to_sector'
616 | static inline sector_t to_sector(unsigned long long n)
|^
In file included from drivers/md/dm-snap.c:8:
include/linux/device-mapper.h:616:24: note: previous definition of 'to_sector' 
was here
616 | static inline sector_t to_sector(unsigned long long n)
|^
In file included from drivers/md/dm.h:14,
from drivers/md/dm-snap.c:22:
>> include/linux/device-mapper.h:621:29: error: redefinition of 'to_bytes'
621 | static inline unsigned long to_bytes(sector_t n)
| ^~~~
In file included from drivers/md/dm-snap.c:8:
include/linux/device-mapper.h:621:29: note: previous definition of 'to_bytes' 
was here
621 | static inline unsigned long to_bytes(sector_t n)
| ^~~~
In file included from drivers/md/dm.h:14,
from drivers/md/dm-snap.c:22:
>> include/linux/device-mapper.h:626:2: error: #endif without #if
626 | #endif /* _LINUX_DEVICE_MAPPER_H */
|  ^
In file included from drivers/md/dm-exception-store.h:15,
from drivers/md/dm-snap.c:24:
>> include/linux/device-mapper.h:616:24: error: redefinition of 'to_sector'
616 | static inline sector_t to_sector(unsigned long long n)
|^
In file included from drivers/md/dm.h:14,
from drivers/md/dm-snap.c:22:
include/linux/device-mapper.h:616:24: note: previous definition of 'to_sector' 
was here
616 | static inline sector_t to_sector(unsigned long long n)
|^
In file included from drivers/md/dm-exception-store.h:15,
from drivers/md/dm-snap.c:24:
>> include/linux/device-mapper.h:621:29: error: redefinition of 'to_bytes'
621 | static inline unsigned long to_bytes(sector_t n)
| ^~~~
In file included from drivers/md/dm.h:14,
from drivers/md/dm-snap.c:22:
include/linux/device-mapper.h:621:29: note: previous definition of 'to_bytes' 
was here
621 | static inline unsigned long to_bytes(sector_t n)
| ^~~~
In file included from drivers/md/dm-exception-store.h:15,
from drivers/md/dm-snap.c:24:
>> include/linux/device-mapper.h:626:2: error: #endif without #if
626 | #endif /* _LINUX_DEVICE_MAPPER_H */
|  ^
--
In file included from drivers/md/dm-zoned.h:13,
from drivers/md/dm-zoned-metadata.c:8:
>>

Re: [dm-devel] [PATCH 1/2] device-mapper: use dynamic debug instead of compile-time config option

2020-05-14 Thread Mike Snitzer 
On Thu, May 14 2020 at  2:09am -0400,
Hannes Reinecke  wrote:

> Switch to use dynamic debug to avoid having recompile the kernel
> just to enable debugging messages.
> 
> Signed-off-by: Hannes Reinecke 

FYI, I decided to always use pr_debug* rather than preserve using printk
if CONFIG_DM_DEBUG is enabled.  More consistent and less surprising for
a developer (to have to reason through how to enable debugging).

May cause initial surprise if debugging messages disappear despite
CONFIG_DM_DEBUG being enabled... but pretty sure DM developers can cope
(and/or help non-developers) as needed.

Patches were staged for 5.8, I also added an additional related cleanup:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.8

Thanks,
Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/2] device-mapper: use dynamic debug instead of compile-time config option

2020-05-14 Thread Mike Snitzer 
On Thu, May 14 2020 at  3:53am -0400,
Damien Le Moal  wrote:

> On 2020/05/14 15:09, Hannes Reinecke wrote:
> > Switch to use dynamic debug to avoid having recompile the kernel
> > just to enable debugging messages.
> > 
> > Signed-off-by: Hannes Reinecke 
> > ---
> >  include/linux/device-mapper.h | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> > index af48d9da3916..4694e1bb4196 100644
> > --- a/include/linux/device-mapper.h
> > +++ b/include/linux/device-mapper.h
> > @@ -557,12 +557,11 @@ void *dm_vcalloc(unsigned long nmemb, unsigned long 
> > elem_size);
> >  #define DMINFO(fmt, ...) pr_info(DM_FMT(fmt), ##__VA_ARGS__)
> >  #define DMINFO_LIMIT(fmt, ...) pr_info_ratelimited(DM_FMT(fmt), 
> > ##__VA_ARGS__)
> >  
> > +#define DMDEBUG_LIMIT(fmt, ...) pr_debug_ratelimited(DM_FMT(fmt), 
> > ##__VA_ARGS__)
> 
> Why do you move this one out of the #ifdef CONFIG_DM_DEBUG scope ?

I'd imagine because it already uses dynamic debugging and is useful even
if CONFIG_DM_DEBUG isn't set.  Makes sense to me.  I'll add a note about
it in the commit header though.

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 2/2] dm-zoned: remove spurious newlines from debugging messages

2020-05-14 Thread Damien Le Moal 
On 2020/05/14 15:09, Hannes Reinecke wrote:
> DMDEBUG will already add a newline to the logging messages, so we
> shouldn't be adding it to the message itself.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/md/dm-zoned-reclaim.c | 4 ++--
>  drivers/md/dm-zoned-target.c  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-zoned-reclaim.c b/drivers/md/dm-zoned-reclaim.c
> index 39ea0d5d4706..4bfa61540b9c 100644
> --- a/drivers/md/dm-zoned-reclaim.c
> +++ b/drivers/md/dm-zoned-reclaim.c
> @@ -405,7 +405,7 @@ static int dmz_do_reclaim(struct dmz_reclaim *zrc)
>  
>   ret = dmz_flush_metadata(zrc->metadata);
>   if (ret) {
> - DMDEBUG("(%s): Metadata flush for zone %u failed, err %d\n",
> + DMDEBUG("(%s): Metadata flush for zone %u failed, err %d",
>   dmz_metadata_label(zmd), rzone->id, ret);
>   return ret;
>   }
> @@ -493,7 +493,7 @@ static void dmz_reclaim_work(struct work_struct *work)
>  
>   ret = dmz_do_reclaim(zrc);
>   if (ret) {
> - DMDEBUG("(%s): Reclaim error %d\n",
> + DMDEBUG("(%s): Reclaim error %d",
>   dmz_metadata_label(zmd), ret);
>   if (!dmz_check_dev(zmd))
>   return;
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index ea43f6892ced..a3d572da70ad 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -515,7 +515,7 @@ static void dmz_flush_work(struct work_struct *work)
>   /* Flush dirty metadata blocks */
>   ret = dmz_flush_metadata(dmz->metadata);
>   if (ret)
> - DMDEBUG("(%s): Metadata flush failed, rc=%d\n",
> + DMDEBUG("(%s): Metadata flush failed, rc=%d",
>   dmz_metadata_label(dmz->metadata), ret);
>  
>   /* Process queued flush requests */
> @@ -679,7 +679,7 @@ static int dmz_map(struct dm_target *ti, struct bio *bio)
>   /* Now ready to handle this BIO */
>   ret = dmz_queue_chunk_work(dmz, bio);
>   if (ret) {
> - DMDEBUG("(%s): BIO op %d, can't process chunk %llu, err %i\n",
> + DMDEBUG("(%s): BIO op %d, can't process chunk %llu, err %i",
>   dmz_metadata_label(zmd),
>   bio_op(bio), (u64)dmz_bio_chunk(zmd, bio),
>   ret);
> 

Reviewed-by: Damien Le Moal 

-- 
Damien Le Moal
Western Digital Research



--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/2] device-mapper: use dynamic debug instead of compile-time config option

2020-05-14 Thread Damien Le Moal 
On 2020/05/14 15:09, Hannes Reinecke wrote:
> Switch to use dynamic debug to avoid having recompile the kernel
> just to enable debugging messages.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  include/linux/device-mapper.h | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index af48d9da3916..4694e1bb4196 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -557,12 +557,11 @@ void *dm_vcalloc(unsigned long nmemb, unsigned long 
> elem_size);
>  #define DMINFO(fmt, ...) pr_info(DM_FMT(fmt), ##__VA_ARGS__)
>  #define DMINFO_LIMIT(fmt, ...) pr_info_ratelimited(DM_FMT(fmt), 
> ##__VA_ARGS__)
>  
> +#define DMDEBUG_LIMIT(fmt, ...) pr_debug_ratelimited(DM_FMT(fmt), 
> ##__VA_ARGS__)

Why do you move this one out of the #ifdef CONFIG_DM_DEBUG scope ?

>  #ifdef CONFIG_DM_DEBUG
>  #define DMDEBUG(fmt, ...) printk(KERN_DEBUG DM_FMT(fmt), ##__VA_ARGS__)
> -#define DMDEBUG_LIMIT(fmt, ...) pr_debug_ratelimited(DM_FMT(fmt), 
> ##__VA_ARGS__)
>  #else
> -#define DMDEBUG(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> -#define DMDEBUG_LIMIT(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> +#define DMDEBUG(fmt, ...) pr_debug(DM_FMT(fmt), ##__VA_ARGS__)
>  #endif
>  
>  #define DMEMIT(x...) sz += ((sz >= maxlen) ? \
> 


-- 
Damien Le Moal
Western Digital Research



--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH] block: Improve io_opt limit stacking

2020-05-14 Thread Damien Le Moal 
When devices with different physical sector sizes are stacked, the
largest value is used as the stacked device physical sector size. For
the optimal IO size, the lowest common multiple (lcm) of the underlying
devices is used for the stacked device. In this scenario, if only one of
the underlying device reports an optimal IO size, that value is used as
is for the stacked device but that value may not be a multiple of the
stacked device physical sector size. In this case, blk_stack_limits()
returns an error resulting in warnings being printed on device mapper
startup (observed with dm-zoned dual drive setup combining a 512B
sector SSD with a 4K sector HDD).

To fix this, rather than returning an error, the optimal IO size limit
for the stacked device can be adjusted to the lowest common multiple
(lcm) of the stacked physical sector size and optimal IO size, resulting
in a value that is a multiple of the physical sector size while still
being an optimal size for the underlying devices.

This patch is complementary to the patch "nvme: Fix io_opt limit
setting" which prevents the nvme driver from reporting an optimal IO
size equal to a namespace sector size for a device that does not report
an optimal IO size.

Suggested-by: Keith Busch 
Signed-off-by: Damien Le Moal 
---
 block/blk-settings.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 9a2c23cd9700..9a2b017ff681 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -561,11 +561,8 @@ int blk_stack_limits(struct queue_limits *t, struct 
queue_limits *b,
}
 
/* Optimal I/O a multiple of the physical block size? */
-   if (t->io_opt & (t->physical_block_size - 1)) {
-   t->io_opt = 0;
-   t->misaligned = 1;
-   ret = -1;
-   }
+   if (t->io_opt & (t->physical_block_size - 1))
+   t->io_opt = lcm(t->io_opt, t->physical_block_size);
 
t->raid_partial_stripes_expensive =
max(t->raid_partial_stripes_expensive,
-- 
2.25.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 2/2] dm-zoned: split off random and cache zones

2020-05-14 Thread Hannes Reinecke 

On 5/14/20 8:45 AM, Damien Le Moal wrote:

On 2020/05/14 15:41, Hannes Reinecke wrote:

On 5/13/20 2:44 PM, Damien Le Moal wrote:

On 2020/05/13 16:07, Hannes Reinecke wrote:

Instead of emulating zones on the regular disk as random zones
this patch adds a new 'cache' zone type.
This allows us to use the random zones on the zoned disk as
data zones (if cache zones are present), and improves performance
as the zones on the (slower) zoned disk are then never used
for caching.

Signed-off-by: Hannes Reinecke 
---

[ .. ]

@@ -1830,17 +1861,19 @@ static void dmz_wait_for_reclaim(struct dmz_metadata 
*zmd, struct dm_zone *zone)
   }
   
   /*

- * Select a random write zone for reclaim.
+ * Select a cache or random write zone for reclaim.
*/
   static struct dm_zone *dmz_get_rnd_zone_for_reclaim(struct dmz_metadata *zmd)
   {
struct dm_zone *dzone = NULL;
struct dm_zone *zone;
+   struct list_head *zone_list = >map_rnd_list;
   
-	if (list_empty(>map_rnd_list))

-   return ERR_PTR(-EBUSY);
+   /* If we have cache zones select from the cache zone list */
+   if (zmd->nr_cache)
+   zone_list = >map_cache_list;
   
-	list_for_each_entry(zone, >map_rnd_list, link) {

+   list_for_each_entry(zone, zone_list, link) {
if (dmz_is_buf(zone))
dzone = zone->bzone;
else
@@ -1853,15 +1886,21 @@ static struct dm_zone 
*dmz_get_rnd_zone_for_reclaim(struct dmz_metadata *zmd)
   }
   
   /*

- * Select a buffered sequential zone for reclaim.
+ * Select a buffered random write or sequential zone for reclaim.


Random write zoned should never be "buffered", or to be very precise, they will
be only during the time reclaim moves a cache zone data to a random zone. That
is visible in the dmz_handle_write() change that execute
dmz_handle_direct_write() for cache or buffered zones instead of using
dmz_handle_buffered_write(). So I think this comment can stay as is.


*/
   static struct dm_zone *dmz_get_seq_zone_for_reclaim(struct dmz_metadata *zmd)
   {
struct dm_zone *zone;
   
-	if (list_empty(>map_seq_list))

-   return ERR_PTR(-EBUSY);
-
+   if (zmd->nr_cache) {
+   /* If we have cache zones start with random zones */
+   list_for_each_entry(zone, >map_rnd_list, link) {
+   if (!zone->bzone)
+   continue;
+   if (dmz_lock_zone_reclaim(zone))
+   return zone;
+   }
+   }


For the reason stated above, I think this change is not necessary either.


Ah. Indeed. The above hunk makes us reclaim the random zones, too, which
strictly speaking isn't necessary.
I'll be dropping it and see how things behave.


list_for_each_entry(zone, >map_seq_list, link) {
if (!zone->bzone)
continue;
@@ -1911,6 +1950,7 @@ struct dm_zone *dmz_get_chunk_mapping(struct dmz_metadata 
*zmd, unsigned int chu
unsigned int dzone_id;
struct dm_zone *dzone = NULL;
int ret = 0;
+   int alloc_flags = zmd->nr_cache ? DMZ_ALLOC_CACHE : DMZ_ALLOC_RND;
   
   	dmz_lock_map(zmd);

   again:
@@ -1925,7 +1965,7 @@ struct dm_zone *dmz_get_chunk_mapping(struct dmz_metadata 
*zmd, unsigned int chu
goto out;
   
   		/* Allocate a random zone */

-   dzone = dmz_alloc_zone(zmd, DMZ_ALLOC_RND);
+   dzone = dmz_alloc_zone(zmd, alloc_flags);
if (!dzone) {
if (dmz_dev_is_dying(zmd)) {
dzone = ERR_PTR(-EIO);
@@ -2018,6 +2058,7 @@ struct dm_zone *dmz_get_chunk_buffer(struct dmz_metadata 
*zmd,
 struct dm_zone *dzone)
   {
struct dm_zone *bzone;
+   int alloc_flags = zmd->nr_cache ? DMZ_ALLOC_CACHE : DMZ_ALLOC_RND;
   
   	dmz_lock_map(zmd);

   again:
@@ -2026,7 +2067,7 @@ struct dm_zone *dmz_get_chunk_buffer(struct dmz_metadata 
*zmd,
goto out;
   
   	/* Allocate a random zone */

-   bzone = dmz_alloc_zone(zmd, DMZ_ALLOC_RND);
+   bzone = dmz_alloc_zone(zmd, alloc_flags);
if (!bzone) {
if (dmz_dev_is_dying(zmd)) {
bzone = ERR_PTR(-EIO);
@@ -2043,7 +2084,10 @@ struct dm_zone *dmz_get_chunk_buffer(struct dmz_metadata 
*zmd,
bzone->chunk = dzone->chunk;
bzone->bzone = dzone;
dzone->bzone = bzone;
-   list_add_tail(>link, >map_rnd_list);
+   if (alloc_flags == DMZ_ALLOC_CACHE)
+   list_add_tail(>link, >map_cache_list);
+   else
+   list_add_tail(>link, >map_rnd_list);
   out:
dmz_unlock_map(zmd);
   
@@ -2059,31 +2103,53 @@ struct dm_zone *dmz_alloc_zone(struct dmz_metadata *zmd, unsigned long flags)

struct list_head *list;
struct dm_zone *zone;
   
-	if (flags & DMZ_ALLOC_RND)

+   switch (flags) {
+

Re: [dm-devel] [PATCH 2/2] dm-zoned: split off random and cache zones

2020-05-14 Thread Damien Le Moal 
On 2020/05/14 15:41, Hannes Reinecke wrote:
> On 5/13/20 2:44 PM, Damien Le Moal wrote:
>> On 2020/05/13 16:07, Hannes Reinecke wrote:
>>> Instead of emulating zones on the regular disk as random zones
>>> this patch adds a new 'cache' zone type.
>>> This allows us to use the random zones on the zoned disk as
>>> data zones (if cache zones are present), and improves performance
>>> as the zones on the (slower) zoned disk are then never used
>>> for caching.
>>>
>>> Signed-off-by: Hannes Reinecke 
>>> ---
> [ .. ]
>>> @@ -1830,17 +1861,19 @@ static void dmz_wait_for_reclaim(struct 
>>> dmz_metadata *zmd, struct dm_zone *zone)
>>>   }
>>>   
>>>   /*
>>> - * Select a random write zone for reclaim.
>>> + * Select a cache or random write zone for reclaim.
>>>*/
>>>   static struct dm_zone *dmz_get_rnd_zone_for_reclaim(struct dmz_metadata 
>>> *zmd)
>>>   {
>>> struct dm_zone *dzone = NULL;
>>> struct dm_zone *zone;
>>> +   struct list_head *zone_list = >map_rnd_list;
>>>   
>>> -   if (list_empty(>map_rnd_list))
>>> -   return ERR_PTR(-EBUSY);
>>> +   /* If we have cache zones select from the cache zone list */
>>> +   if (zmd->nr_cache)
>>> +   zone_list = >map_cache_list;
>>>   
>>> -   list_for_each_entry(zone, >map_rnd_list, link) {
>>> +   list_for_each_entry(zone, zone_list, link) {
>>> if (dmz_is_buf(zone))
>>> dzone = zone->bzone;
>>> else
>>> @@ -1853,15 +1886,21 @@ static struct dm_zone 
>>> *dmz_get_rnd_zone_for_reclaim(struct dmz_metadata *zmd)
>>>   }
>>>   
>>>   /*
>>> - * Select a buffered sequential zone for reclaim.
>>> + * Select a buffered random write or sequential zone for reclaim.
>>
>> Random write zoned should never be "buffered", or to be very precise, they 
>> will
>> be only during the time reclaim moves a cache zone data to a random zone. 
>> That
>> is visible in the dmz_handle_write() change that execute
>> dmz_handle_direct_write() for cache or buffered zones instead of using
>> dmz_handle_buffered_write(). So I think this comment can stay as is.
>>
>>>*/
>>>   static struct dm_zone *dmz_get_seq_zone_for_reclaim(struct dmz_metadata 
>>> *zmd)
>>>   {
>>> struct dm_zone *zone;
>>>   
>>> -   if (list_empty(>map_seq_list))
>>> -   return ERR_PTR(-EBUSY);
>>> -
>>> +   if (zmd->nr_cache) {
>>> +   /* If we have cache zones start with random zones */
>>> +   list_for_each_entry(zone, >map_rnd_list, link) {
>>> +   if (!zone->bzone)
>>> +   continue;
>>> +   if (dmz_lock_zone_reclaim(zone))
>>> +   return zone;
>>> +   }
>>> +   }
>>
>> For the reason stated above, I think this change is not necessary either.
>>
> Ah. Indeed. The above hunk makes us reclaim the random zones, too, which
> strictly speaking isn't necessary.
> I'll be dropping it and see how things behave.
> 
>>> list_for_each_entry(zone, >map_seq_list, link) {
>>> if (!zone->bzone)
>>> continue;
>>> @@ -1911,6 +1950,7 @@ struct dm_zone *dmz_get_chunk_mapping(struct 
>>> dmz_metadata *zmd, unsigned int chu
>>> unsigned int dzone_id;
>>> struct dm_zone *dzone = NULL;
>>> int ret = 0;
>>> +   int alloc_flags = zmd->nr_cache ? DMZ_ALLOC_CACHE : DMZ_ALLOC_RND;
>>>   
>>> dmz_lock_map(zmd);
>>>   again:
>>> @@ -1925,7 +1965,7 @@ struct dm_zone *dmz_get_chunk_mapping(struct 
>>> dmz_metadata *zmd, unsigned int chu
>>> goto out;
>>>   
>>> /* Allocate a random zone */
>>> -   dzone = dmz_alloc_zone(zmd, DMZ_ALLOC_RND);
>>> +   dzone = dmz_alloc_zone(zmd, alloc_flags);
>>> if (!dzone) {
>>> if (dmz_dev_is_dying(zmd)) {
>>> dzone = ERR_PTR(-EIO);
>>> @@ -2018,6 +2058,7 @@ struct dm_zone *dmz_get_chunk_buffer(struct 
>>> dmz_metadata *zmd,
>>>  struct dm_zone *dzone)
>>>   {
>>> struct dm_zone *bzone;
>>> +   int alloc_flags = zmd->nr_cache ? DMZ_ALLOC_CACHE : DMZ_ALLOC_RND;
>>>   
>>> dmz_lock_map(zmd);
>>>   again:
>>> @@ -2026,7 +2067,7 @@ struct dm_zone *dmz_get_chunk_buffer(struct 
>>> dmz_metadata *zmd,
>>> goto out;
>>>   
>>> /* Allocate a random zone */
>>> -   bzone = dmz_alloc_zone(zmd, DMZ_ALLOC_RND);
>>> +   bzone = dmz_alloc_zone(zmd, alloc_flags);
>>> if (!bzone) {
>>> if (dmz_dev_is_dying(zmd)) {
>>> bzone = ERR_PTR(-EIO);
>>> @@ -2043,7 +2084,10 @@ struct dm_zone *dmz_get_chunk_buffer(struct 
>>> dmz_metadata *zmd,
>>> bzone->chunk = dzone->chunk;
>>> bzone->bzone = dzone;
>>> dzone->bzone = bzone;
>>> -   list_add_tail(>link, >map_rnd_list);
>>> +   if (alloc_flags == DMZ_ALLOC_CACHE)
>>> +   list_add_tail(>link, >map_cache_list);
>>> +   else
>>> +   list_add_tail(>link, >map_rnd_list);
>>>   out:
>>> dmz_unlock_map(zmd);
>>>   
>>> @@

Re: [dm-devel] [PATCH 2/2] dm-zoned: split off random and cache zones

2020-05-14 Thread Hannes Reinecke 

On 5/13/20 2:44 PM, Damien Le Moal wrote:

On 2020/05/13 16:07, Hannes Reinecke wrote:

Instead of emulating zones on the regular disk as random zones
this patch adds a new 'cache' zone type.
This allows us to use the random zones on the zoned disk as
data zones (if cache zones are present), and improves performance
as the zones on the (slower) zoned disk are then never used
for caching.

Signed-off-by: Hannes Reinecke 
---

[ .. ]

@@ -1830,17 +1861,19 @@ static void dmz_wait_for_reclaim(struct dmz_metadata 
*zmd, struct dm_zone *zone)
  }
  
  /*

- * Select a random write zone for reclaim.
+ * Select a cache or random write zone for reclaim.
   */
  static struct dm_zone *dmz_get_rnd_zone_for_reclaim(struct dmz_metadata *zmd)
  {
struct dm_zone *dzone = NULL;
struct dm_zone *zone;
+   struct list_head *zone_list = >map_rnd_list;
  
-	if (list_empty(>map_rnd_list))

-   return ERR_PTR(-EBUSY);
+   /* If we have cache zones select from the cache zone list */
+   if (zmd->nr_cache)
+   zone_list = >map_cache_list;
  
-	list_for_each_entry(zone, >map_rnd_list, link) {

+   list_for_each_entry(zone, zone_list, link) {
if (dmz_is_buf(zone))
dzone = zone->bzone;
else
@@ -1853,15 +1886,21 @@ static struct dm_zone 
*dmz_get_rnd_zone_for_reclaim(struct dmz_metadata *zmd)
  }
  
  /*

- * Select a buffered sequential zone for reclaim.
+ * Select a buffered random write or sequential zone for reclaim.


Random write zoned should never be "buffered", or to be very precise, they will
be only during the time reclaim moves a cache zone data to a random zone. That
is visible in the dmz_handle_write() change that execute
dmz_handle_direct_write() for cache or buffered zones instead of using
dmz_handle_buffered_write(). So I think this comment can stay as is.


   */
  static struct dm_zone *dmz_get_seq_zone_for_reclaim(struct dmz_metadata *zmd)
  {
struct dm_zone *zone;
  
-	if (list_empty(>map_seq_list))

-   return ERR_PTR(-EBUSY);
-
+   if (zmd->nr_cache) {
+   /* If we have cache zones start with random zones */
+   list_for_each_entry(zone, >map_rnd_list, link) {
+   if (!zone->bzone)
+   continue;
+   if (dmz_lock_zone_reclaim(zone))
+   return zone;
+   }
+   }


For the reason stated above, I think this change is not necessary either.


Ah. Indeed. The above hunk makes us reclaim the random zones, too, which
strictly speaking isn't necessary.
I'll be dropping it and see how things behave.


list_for_each_entry(zone, >map_seq_list, link) {
if (!zone->bzone)
continue;
@@ -1911,6 +1950,7 @@ struct dm_zone *dmz_get_chunk_mapping(struct dmz_metadata 
*zmd, unsigned int chu
unsigned int dzone_id;
struct dm_zone *dzone = NULL;
int ret = 0;
+   int alloc_flags = zmd->nr_cache ? DMZ_ALLOC_CACHE : DMZ_ALLOC_RND;
  
  	dmz_lock_map(zmd);

  again:
@@ -1925,7 +1965,7 @@ struct dm_zone *dmz_get_chunk_mapping(struct dmz_metadata 
*zmd, unsigned int chu
goto out;
  
  		/* Allocate a random zone */

-   dzone = dmz_alloc_zone(zmd, DMZ_ALLOC_RND);
+   dzone = dmz_alloc_zone(zmd, alloc_flags);
if (!dzone) {
if (dmz_dev_is_dying(zmd)) {
dzone = ERR_PTR(-EIO);
@@ -2018,6 +2058,7 @@ struct dm_zone *dmz_get_chunk_buffer(struct dmz_metadata 
*zmd,
 struct dm_zone *dzone)
  {
struct dm_zone *bzone;
+   int alloc_flags = zmd->nr_cache ? DMZ_ALLOC_CACHE : DMZ_ALLOC_RND;
  
  	dmz_lock_map(zmd);

  again:
@@ -2026,7 +2067,7 @@ struct dm_zone *dmz_get_chunk_buffer(struct dmz_metadata 
*zmd,
goto out;
  
  	/* Allocate a random zone */

-   bzone = dmz_alloc_zone(zmd, DMZ_ALLOC_RND);
+   bzone = dmz_alloc_zone(zmd, alloc_flags);
if (!bzone) {
if (dmz_dev_is_dying(zmd)) {
bzone = ERR_PTR(-EIO);
@@ -2043,7 +2084,10 @@ struct dm_zone *dmz_get_chunk_buffer(struct dmz_metadata 
*zmd,
bzone->chunk = dzone->chunk;
bzone->bzone = dzone;
dzone->bzone = bzone;
-   list_add_tail(>link, >map_rnd_list);
+   if (alloc_flags == DMZ_ALLOC_CACHE)
+   list_add_tail(>link, >map_cache_list);
+   else
+   list_add_tail(>link, >map_rnd_list);
  out:
dmz_unlock_map(zmd);
  
@@ -2059,31 +2103,53 @@ struct dm_zone *dmz_alloc_zone(struct dmz_metadata *zmd, unsigned long flags)

struct list_head *list;
struct dm_zone *zone;
  
-	if (flags & DMZ_ALLOC_RND)

+   switch (flags) {
+   case DMZ_ALLOC_CACHE:
+   list = >unmap_cache_list;
+   break;
+   case DMZ_ALLOC_RND:

Re: [dm-devel] [PATCH] dm zoned: Avoid 64-bit division error in dmz_fixup_devices

2020-05-14 Thread Damien Le Moal 
On 2020/05/14 14:07, Nathan Chancellor wrote:
> When building arm32 allyesconfig:
> 
> ld.lld: error: undefined symbol: __aeabi_uldivmod
 referenced by dm-zoned-target.c
   md/dm-zoned-target.o:(dmz_ctr) in archive drivers/built-in.a
> 
> dmz_fixup_devices uses DIV_ROUND_UP with variables of type sector_t. As
> such, it should be using DIV_ROUND_UP_SECTOR_T, which handles this
> automatically.
> 
> Fixes: 70978208ec91 ("dm zoned: metadata version 2")
> Signed-off-by: Nathan Chancellor 
> ---
>  drivers/md/dm-zoned-target.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index ea43f6892ced..9c4fd4b04878 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -803,8 +803,9 @@ static int dmz_fixup_devices(struct dm_target *ti)
>  
>   if (reg_dev) {
>   reg_dev->zone_nr_sectors = zoned_dev->zone_nr_sectors;
> - reg_dev->nr_zones = DIV_ROUND_UP(reg_dev->capacity,
> -  reg_dev->zone_nr_sectors);
> + reg_dev->nr_zones =
> + DIV_ROUND_UP_SECTOR_T(reg_dev->capacity,
> +   reg_dev->zone_nr_sectors);>   
> zoned_dev->zone_offset = reg_dev->nr_zones;
>   }
>   return 0;
> 
> base-commit: e098d7762d602be640c53565ceca342f81e55ad2
> 

Looks good.

Reviewed-by: Damien Le Moal 

-- 
Damien Le Moal
Western Digital Research



--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 2/2] dm-zoned: remove spurious newlines from debugging messages

2020-05-14 Thread Hannes Reinecke 
DMDEBUG will already add a newline to the logging messages, so we
shouldn't be adding it to the message itself.

Signed-off-by: Hannes Reinecke 
---
 drivers/md/dm-zoned-reclaim.c | 4 ++--
 drivers/md/dm-zoned-target.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-zoned-reclaim.c b/drivers/md/dm-zoned-reclaim.c
index 39ea0d5d4706..4bfa61540b9c 100644
--- a/drivers/md/dm-zoned-reclaim.c
+++ b/drivers/md/dm-zoned-reclaim.c
@@ -405,7 +405,7 @@ static int dmz_do_reclaim(struct dmz_reclaim *zrc)
 
ret = dmz_flush_metadata(zrc->metadata);
if (ret) {
-   DMDEBUG("(%s): Metadata flush for zone %u failed, err %d\n",
+   DMDEBUG("(%s): Metadata flush for zone %u failed, err %d",
dmz_metadata_label(zmd), rzone->id, ret);
return ret;
}
@@ -493,7 +493,7 @@ static void dmz_reclaim_work(struct work_struct *work)
 
ret = dmz_do_reclaim(zrc);
if (ret) {
-   DMDEBUG("(%s): Reclaim error %d\n",
+   DMDEBUG("(%s): Reclaim error %d",
dmz_metadata_label(zmd), ret);
if (!dmz_check_dev(zmd))
return;
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index ea43f6892ced..a3d572da70ad 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -515,7 +515,7 @@ static void dmz_flush_work(struct work_struct *work)
/* Flush dirty metadata blocks */
ret = dmz_flush_metadata(dmz->metadata);
if (ret)
-   DMDEBUG("(%s): Metadata flush failed, rc=%d\n",
+   DMDEBUG("(%s): Metadata flush failed, rc=%d",
dmz_metadata_label(dmz->metadata), ret);
 
/* Process queued flush requests */
@@ -679,7 +679,7 @@ static int dmz_map(struct dm_target *ti, struct bio *bio)
/* Now ready to handle this BIO */
ret = dmz_queue_chunk_work(dmz, bio);
if (ret) {
-   DMDEBUG("(%s): BIO op %d, can't process chunk %llu, err %i\n",
+   DMDEBUG("(%s): BIO op %d, can't process chunk %llu, err %i",
dmz_metadata_label(zmd),
bio_op(bio), (u64)dmz_bio_chunk(zmd, bio),
ret);
-- 
2.16.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCHv2 0/2] device-mapper: use dynamic debug

2020-05-14 Thread Hannes Reinecke 
Hi all,

here's an updated version of my earlier patch to switch to dynamic
debug instead of using a compile-time option.
I've modified the initial patch as suggested by Mike, and also fixed
up stray newlines which had crept in with some debugging messages
in dm-zoned.

Hannes Reinecke (2):
  device-mapper: use dynamic debug instead of compile-time config option
  dm-zoned: remove spurious newlines from debugging messages

 drivers/md/dm-zoned-reclaim.c | 4 ++--
 drivers/md/dm-zoned-target.c  | 4 ++--
 include/linux/device-mapper.h | 5 ++---
 3 files changed, 6 insertions(+), 7 deletions(-)

-- 
2.16.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 1/2] device-mapper: use dynamic debug instead of compile-time config option

2020-05-14 Thread Hannes Reinecke 
Switch to use dynamic debug to avoid having recompile the kernel
just to enable debugging messages.

Signed-off-by: Hannes Reinecke 
---
 include/linux/device-mapper.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index af48d9da3916..4694e1bb4196 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -557,12 +557,11 @@ void *dm_vcalloc(unsigned long nmemb, unsigned long 
elem_size);
 #define DMINFO(fmt, ...) pr_info(DM_FMT(fmt), ##__VA_ARGS__)
 #define DMINFO_LIMIT(fmt, ...) pr_info_ratelimited(DM_FMT(fmt), ##__VA_ARGS__)
 
+#define DMDEBUG_LIMIT(fmt, ...) pr_debug_ratelimited(DM_FMT(fmt), 
##__VA_ARGS__)
 #ifdef CONFIG_DM_DEBUG
 #define DMDEBUG(fmt, ...) printk(KERN_DEBUG DM_FMT(fmt), ##__VA_ARGS__)
-#define DMDEBUG_LIMIT(fmt, ...) pr_debug_ratelimited(DM_FMT(fmt), 
##__VA_ARGS__)
 #else
-#define DMDEBUG(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
-#define DMDEBUG_LIMIT(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
+#define DMDEBUG(fmt, ...) pr_debug(DM_FMT(fmt), ##__VA_ARGS__)
 #endif
 
 #define DMEMIT(x...) sz += ((sz >= maxlen) ? \
-- 
2.16.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel