All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency
@ 2017-09-21 13:23 Guan Junxiong
  2017-09-21 13:23 ` [PATCH V2 1/4] multipath-tools: use direct IO for path latency prioritizer Guan Junxiong
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Guan Junxiong @ 2017-09-21 13:23 UTC (permalink / raw)
  To: christophe.varoqui, dm-devel, hare
  Cc: xose.vazquez, guanjunxiong, chengjike.cheng, shenhong09,
	niuhaoxin, mwilck

Hi Christophe,

Please consider this series of patches.

This series of patches help to make IO processing more common for
path-latency prioritizer, make the multipath.conf more user-friendly
and fixup the error of calculation on log scale standard deviation.  

First, the SCSI-to-NVMe translations which was blamed broken has been removed
since linux kernel 4.13, so that SG_IO IOCTL used in the reading is not
supported. Instead, PATCH 1/4 drops sg_read method and uses direct IO
reading both for NVMe device and SCSI device.

Second, the original prio_args for prioritizer is like this: 20|10 which
is somewhat unconvenient for user. PATCH 3/4 drops it and use a syntax that
similar to other prioritizers, for example :
"base_num=5 io_num=10".

PATCH 2/4 can be an independent patch.

Third, Martin has pointed that standard deviation in the path-latency prioritizer
is wrong. We have solved this and here come the PATCH 4/4.  


Thanks
Guan Junxiong


Changes from V1:
* add PATCH 4/4 
* rebase on the 0.7.3 tag

Junxiong Guan (3):
  multipath-tools: use direct IO for path latency prioritizer
  multipath-tools: move get_next_string to util
  multipath-tools: use user-friendly prio_args for path-latency

 libmultipath/prioritizers/path_latency.c | 156 +++++++++++++++++++++++--------
 libmultipath/prioritizers/weightedpath.c |  10 +-
 libmultipath/util.c                      |   9 ++
 libmultipath/util.h                      |   1 +
 multipath/multipath.conf.5               |   2 +-
 5 files changed, 127 insertions(+), 51 deletions(-)

Guan Junxiong (4):
  multipath-tools: use direct IO for path latency prioritizer
  multipath-tools: move get_next_string to util
  multipath-tools: use user-friendly prio_args for path-latency
  multipath-tools: calculate standard deviation on a logarithmic scale
    for prioritizer path_latency

 libmultipath/prioritizers/path_latency.c | 321 ++++++++++++++++++++-----------
 libmultipath/prioritizers/weightedpath.c |  10 +-
 libmultipath/util.c                      |   9 +
 libmultipath/util.h                      |   1 +
 multipath/multipath.conf.5               |   2 +-
 5 files changed, 217 insertions(+), 126 deletions(-)

-- 
2.11.1

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

* [PATCH V2 1/4] multipath-tools: use direct IO for path latency prioritizer
  2017-09-21 13:23 [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
@ 2017-09-21 13:23 ` Guan Junxiong
  2017-09-21 13:23 ` [PATCH V2 2/4] multipath-tools: move get_next_string to util Guan Junxiong
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guan Junxiong @ 2017-09-21 13:23 UTC (permalink / raw)
  To: christophe.varoqui, dm-devel, hare
  Cc: xose.vazquez, guanjunxiong, chengjike.cheng, shenhong09,
	niuhaoxin, mwilck

The SCSI-to-NVMe translations which was blamed broken has been removed
since linux kernel 4.13, so that SG_IO IOCTL used in the reading is not
supported. Instead, this patch drops sg_read method and uses direct IO
reading both for NVMe device and SCSI device.

Signed-off-by: Junxiong Guan <guanjunxiong@huawei.com>
---
 libmultipath/prioritizers/path_latency.c | 85 +++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 7 deletions(-)

diff --git a/libmultipath/prioritizers/path_latency.c b/libmultipath/prioritizers/path_latency.c
index 9fc2dfc0..c75ae03f 100644
--- a/libmultipath/prioritizers/path_latency.c
+++ b/libmultipath/prioritizers/path_latency.c
@@ -19,15 +19,19 @@
  * This file is released under the GPL version 2, or any later version.
  */
 
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <math.h>
 #include <ctype.h>
 #include <time.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <linux/fs.h>
+#include <unistd.h>
 
 #include "debug.h"
 #include "prio.h"
 #include "structs.h"
-#include "../checkers/libsg.h"
 
 #define pp_pl_log(prio, fmt, args...) condlog(prio, "path_latency prio: " fmt, ##args)
 
@@ -47,6 +51,8 @@
 #define USEC_PER_SEC		1000000LL
 #define NSEC_PER_USEC		1000LL
 
+#define DEF_BLK_SIZE		4096
+
 static long long path_latency[MAX_IO_NUM];
 
 static inline long long timeval_to_us(const struct timespec *tv)
@@ -55,15 +61,72 @@ static inline long long timeval_to_us(const struct timespec *tv)
 	    (tv->tv_nsec / NSEC_PER_USEC);
 }
 
-static int do_readsector0(int fd, unsigned int timeout)
+static int prepare_directio_read(int fd, int *blksz, char **pbuf,
+		int *restore_flags)
+{
+	unsigned long pgsize = getpagesize();
+	long flags;
+
+	if (ioctl(fd, BLKBSZGET, blksz) < 0) {
+		pp_pl_log(3,"catnnot get blocksize, set default");
+		*blksz = DEF_BLK_SIZE;
+	}
+	if (posix_memalign((void **)pbuf, pgsize, *blksz))
+		return -1;
+
+	flags = fcntl(fd, F_GETFL);
+	if (flags < 0)
+		goto free_out;
+	if (!(flags & O_DIRECT)) {
+		flags |= O_DIRECT;
+		if (fcntl(fd, F_SETFL, flags) < 0)
+			goto free_out;
+		*restore_flags = 1;
+	}
+
+	return 0;
+
+free_out:
+	free(*pbuf);
+
+	return -1;
+}
+
+static void cleanup_directio_read(int fd, char *buf, int restore_flags)
 {
-	unsigned char buf[4096];
-	unsigned char sbuf[SENSE_BUFF_LEN];
+	long flags;
+
+	free(buf);
+
+	if (!restore_flags)
+		return;
+	if ((flags = fcntl(fd, F_GETFL)) >= 0) {
+		int ret __attribute__ ((unused));
+		flags &= ~O_DIRECT;
+		/* No point in checking for errors */
+		ret = fcntl(fd, F_SETFL, flags);
+	}
+}
+
+static int do_directio_read(int fd, unsigned int timeout, char *buf, int sz)
+{
+	fd_set read_fds;
+	struct timeval tm = { .tv_sec = timeout };
 	int ret;
+	int num_read;
 
-	ret = sg_read(fd, &buf[0], 4096, &sbuf[0], SENSE_BUFF_LEN, timeout);
+	if (lseek(fd, 0, SEEK_SET) == -1)
+		return -1;
+	FD_ZERO(&read_fds);
+	FD_SET(fd, &read_fds);
+	ret = select(fd+1, &read_fds, NULL, NULL, &tm);
+	if (ret <= 0)
+		return -1;
+	num_read = read(fd, buf, sz);
+	if (num_read != sz)
+		return -1;
 
-	return ret;
+	return 0;
 }
 
 int check_args_valid(int io_num, int base_num)
@@ -194,6 +257,9 @@ int getprio(struct path *pp, char *args, unsigned int timeout)
 	long long toldelay = 0;
 	long long before, after;
 	struct timespec tv;
+	int blksize;
+	char *buf;
+	int restore_flags = 0;
 
 	if (pp->fd < 0)
 		return -1;
@@ -205,13 +271,16 @@ int getprio(struct path *pp, char *args, unsigned int timeout)
 
 	memset(path_latency, 0, sizeof(path_latency));
 
+	prepare_directio_read(pp->fd, &blksize, &buf, &restore_flags);
+
 	temp = io_num;
 	while (temp-- > 0) {
 		(void)clock_gettime(CLOCK_MONOTONIC, &tv);
 		before = timeval_to_us(&tv);
 
-		if (do_readsector0(pp->fd, timeout) == 2) {
+		if (do_directio_read(pp->fd, timeout, buf, blksize)) {
 			pp_pl_log(0, "%s: path down", pp->dev);
+			cleanup_directio_read(pp->fd, buf, restore_flags);
 			return -1;
 		}
 
@@ -222,6 +291,8 @@ int getprio(struct path *pp, char *args, unsigned int timeout)
 		toldelay += path_latency[index++];
 	}
 
+	cleanup_directio_read(pp->fd, buf, restore_flags);
+
 	avglatency = toldelay / (long long)io_num;
 	pp_pl_log(4, "%s: average latency is (%lld us)", pp->dev, avglatency);
 
-- 
2.11.1

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

* [PATCH V2 2/4] multipath-tools: move get_next_string to util
  2017-09-21 13:23 [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
  2017-09-21 13:23 ` [PATCH V2 1/4] multipath-tools: use direct IO for path latency prioritizer Guan Junxiong
@ 2017-09-21 13:23 ` Guan Junxiong
  2017-09-21 13:23 ` [PATCH V2 3/4] multipath-tools: use user-friendly prio_args for path-latency Guan Junxiong
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guan Junxiong @ 2017-09-21 13:23 UTC (permalink / raw)
  To: christophe.varoqui, dm-devel, hare
  Cc: xose.vazquez, guanjunxiong, chengjike.cheng, shenhong09,
	niuhaoxin, mwilck

The helper get_next_string is useful and generic. So move from
exclusive weightedpath module to util module. It will be used
in the next second patch.

Signed-off-by: Junxiong Guan <guanjunxiong@huawei.com>
---
 libmultipath/prioritizers/weightedpath.c | 10 +---------
 libmultipath/util.c                      |  9 +++++++++
 libmultipath/util.h                      |  1 +
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/libmultipath/prioritizers/weightedpath.c b/libmultipath/prioritizers/weightedpath.c
index 34a43a81..e0f3efbb 100644
--- a/libmultipath/prioritizers/weightedpath.c
+++ b/libmultipath/prioritizers/weightedpath.c
@@ -34,15 +34,7 @@
 #include <regex.h>
 #include "structs_vec.h"
 #include "print.h"
-
-char *get_next_string(char **temp, char *split_char)
-{
-	char *token = NULL;
-	token = strsep(temp, split_char);
-	while (token != NULL && !strcmp(token, ""))
-		token = strsep(temp, split_char);
-	return token;
-}
+#include "util.h"
 
 #define CHECK_LEN \
 do { \
diff --git a/libmultipath/util.c b/libmultipath/util.c
index 0800da52..0b43d29d 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -65,6 +65,15 @@ filepresent (char * run) {
 	return 0;
 }
 
+char *get_next_string(char **temp, char *split_char)
+{
+	char *token = NULL;
+	token = strsep(temp, split_char);
+	while (token != NULL && !strcmp(token, ""))
+		token = strsep(temp, split_char);
+	return token;
+}
+
 int
 get_word (char * sentence, char ** word)
 {
diff --git a/libmultipath/util.h b/libmultipath/util.h
index 3dc048e2..51a6d542 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -7,6 +7,7 @@
 size_t strchop(char *);
 int basenamecpy (const char * src, char * dst, int);
 int filepresent (char * run);
+char *get_next_string(char **temp, char *split_char);
 int get_word (char * sentence, char ** word);
 size_t strlcpy(char *dst, const char *src, size_t size);
 size_t strlcat(char *dst, const char *src, size_t size);
-- 
2.11.1

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

* [PATCH V2 3/4] multipath-tools: use user-friendly prio_args for path-latency
  2017-09-21 13:23 [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
  2017-09-21 13:23 ` [PATCH V2 1/4] multipath-tools: use direct IO for path latency prioritizer Guan Junxiong
  2017-09-21 13:23 ` [PATCH V2 2/4] multipath-tools: move get_next_string to util Guan Junxiong
@ 2017-09-21 13:23 ` Guan Junxiong
  2017-09-21 13:23 ` [PATCH V2 4/4] multipath-tools: calculate standard deviation on a logarithmic scale for prioritizer path_latency Guan Junxiong
  2017-10-10 13:28 ` [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
  4 siblings, 0 replies; 6+ messages in thread
From: Guan Junxiong @ 2017-09-21 13:23 UTC (permalink / raw)
  To: christophe.varoqui, dm-devel, hare
  Cc: xose.vazquez, guanjunxiong, chengjike.cheng, shenhong09,
	niuhaoxin, mwilck

The original prio_args for prioritizer is like this: 20|10 which
is somewhat unconvenient for user. This patch drops it and use
a syntax that similar to other prioritizers, for example :
"base_num=5 io_num=10".

Signed-off-by: Junxiong Guan <guanjunxiong@huawei.com>
---
 libmultipath/prioritizers/path_latency.c | 71 +++++++++++++++++---------------
 multipath/multipath.conf.5               |  2 +-
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/libmultipath/prioritizers/path_latency.c b/libmultipath/prioritizers/path_latency.c
index c75ae03f..69c9bd65 100644
--- a/libmultipath/prioritizers/path_latency.c
+++ b/libmultipath/prioritizers/path_latency.c
@@ -15,6 +15,7 @@
  *    scale, the priority "rc" of each path can be provided.
  *
  * Author(s): Yang Feng <philip.yang@huawei.com>
+ * Revised:   Guan Junxiong <guanjunxiong@huawei.com>
  *
  * This file is released under the GPL version 2, or any later version.
  */
@@ -32,6 +33,7 @@
 #include "debug.h"
 #include "prio.h"
 #include "structs.h"
+#include "util.h"
 
 #define pp_pl_log(prio, fmt, args...) condlog(prio, "path_latency prio: " fmt, ##args)
 
@@ -46,8 +48,6 @@
 
 #define DEFAULT_PRIORITY	0
 
-#define MAX_CHAR_SIZE		30
-
 #define USEC_PER_SEC		1000000LL
 #define NSEC_PER_USEC		1000LL
 
@@ -145,52 +145,55 @@ int check_args_valid(int io_num, int base_num)
 }
 
 /*
- * In multipath.conf, args form: io_num|base_num. For example,
- * args is "20|10", this function can get io_num value 20, and
+ * In multipath.conf, args form: io_num=n base_num=m. For example, args are
+ * "io_num=20 base_num=10", this function can get io_num value 20 and
  * base_num value 10.
  */
 static int get_ionum_and_basenum(char *args, int *ionum, int *basenum)
 {
-	char source[MAX_CHAR_SIZE];
-	char vertica = '|';
-	char *endstrbefore = NULL;
-	char *endstrafter = NULL;
-	unsigned int size = strlen(args);
+	char split_char[] = " \t";
+	char *arg, *temp;
+	char *str, *str_inval;
+	int i;
+	int flag_io = 0, flag_base = 0;
 
 	if ((args == NULL) || (ionum == NULL) || (basenum == NULL)) {
 		pp_pl_log(0, "args string is NULL");
 		return 0;
 	}
 
-	if ((size < 1) || (size > MAX_CHAR_SIZE - 1)) {
-		pp_pl_log(0, "args string's size is too long");
-		return 0;
-	}
-
-	memcpy(source, args, size + 1);
-
-	if (!isdigit(source[0])) {
-		pp_pl_log(0, "invalid prio_args format: %s", source);
+	arg = temp = STRDUP(args);
+	if (!arg)
 		return 0;
-	}
 
-	*ionum = (int)strtoul(source, &endstrbefore, 10);
-	if (endstrbefore[0] != vertica) {
-		pp_pl_log(0, "invalid prio_args format: %s", source);
-		return 0;
-	}
-
-	if (!isdigit(endstrbefore[1])) {
-		pp_pl_log(0, "invalid prio_args format: %s", source);
-		return 0;
+	for (i = 0; i < 2; i++) {
+		str = get_next_string(&temp, split_char);
+		if (!str)
+			goto out;
+		if (!strncmp(str, "io_num=", 7) && strlen(str) > 7) {
+			*ionum = (int)strtoul(str + 7, &str_inval, 10);
+			if (str == str_inval)
+				goto out;
+			flag_io = 1;
+		}
+		else if (!strncmp(str, "base_num=", 9) && strlen(str) > 9) {
+			*basenum = (int)strtol(str + 9, &str_inval, 10);
+			if (str == str_inval)
+				goto out;
+			flag_base = 1;
+		}
 	}
 
-	*basenum = (long long)strtol(&endstrbefore[1], &endstrafter, 10);
-	if (check_args_valid(*ionum, *basenum) == 0) {
-		return 0;
-	}
+	if (!flag_io || !flag_base)
+		goto out;
+	if (check_args_valid(*ionum, *basenum) == 0)
+		goto out;
 
+	FREE(arg);
 	return 1;
+out:
+	FREE(arg);
+	return 0;
 }
 
 long long calc_standard_deviation(long long *path_latency, int size,
@@ -249,8 +252,8 @@ int getprio(struct path *pp, char *args, unsigned int timeout)
 {
 	int rc, temp;
 	int index = 0;
-	int io_num;
-	int base_num;
+	int io_num = 0;
+	int base_num = 0;
 	long long avglatency;
 	long long latency_interval;
 	long long standard_deviation;
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 5b6dde71..ad1b8eba 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -351,7 +351,7 @@ these values can be looked up through sysfs or by running \fImultipathd show pat
 .RE
 .TP 12
 .I path_latency
-Needs a value of the form \fI"<io_num>|<base_num>"\fR
+Needs a value of the form "io_num=\fI<20>\fR base_num=\fI<10>\fR"
 .RS
 .TP 8
 .I io_num
-- 
2.11.1

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

* [PATCH V2 4/4] multipath-tools: calculate standard deviation on a logarithmic scale for prioritizer path_latency
  2017-09-21 13:23 [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
                   ` (2 preceding siblings ...)
  2017-09-21 13:23 ` [PATCH V2 3/4] multipath-tools: use user-friendly prio_args for path-latency Guan Junxiong
@ 2017-09-21 13:23 ` Guan Junxiong
  2017-10-10 13:28 ` [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
  4 siblings, 0 replies; 6+ messages in thread
From: Guan Junxiong @ 2017-09-21 13:23 UTC (permalink / raw)
  To: christophe.varoqui, dm-devel, hare
  Cc: xose.vazquez, guanjunxiong, chengjike.cheng, shenhong09,
	niuhaoxin, mwilck

There is a wrong calculation for standard deviation for prioritizers
path_latency. Because the scale is logarithmic, we need to calculate
the standard deviation on a log scale. However, if we first compute
every logarithmic value of each sample of path latency, the standard
deviation calculating is simple as before because the logarithm of
logarithmic normal distribution is normal distribution.

Besides, this patch discards the path_interval calculating because
it is unreasonable to compare two different dimension of datas.
Instead, this patch uses another condition which comparable.

In addition, the minimum base_num is large so that standard deviation
is too small to meet the new condition given that the base_num is set
to the smallest value i.e 2. This patch lets base_num range from 1.01
to 100.

Signed-off-by: Junxiong Guan <guanjunxiong@huawei.com>
---
 libmultipath/prioritizers/path_latency.c | 169 +++++++++++++++++--------------
 1 file changed, 92 insertions(+), 77 deletions(-)

diff --git a/libmultipath/prioritizers/path_latency.c b/libmultipath/prioritizers/path_latency.c
index 69c9bd65..9d5397ec 100644
--- a/libmultipath/prioritizers/path_latency.c
+++ b/libmultipath/prioritizers/path_latency.c
@@ -38,10 +38,12 @@
 #define pp_pl_log(prio, fmt, args...) condlog(prio, "path_latency prio: " fmt, ##args)
 
 #define MAX_IO_NUM		200
-#define MIN_IO_NUM		2
+#define MIN_IO_NUM		20
+#define DEF_IO_NUM		100
 
 #define MAX_BASE_NUM		10
-#define MIN_BASE_NUM		2
+#define MIN_BASE_NUM		1.01
+#define DEF_BASE_NUM		1.5
 
 #define MAX_AVG_LATENCY		100000000.	/* Unit: us */
 #define MIN_AVG_LATENCY		1.		/* Unit: us */
@@ -53,7 +55,7 @@
 
 #define DEF_BLK_SIZE		4096
 
-static long long path_latency[MAX_IO_NUM];
+static double lg_path_latency[MAX_IO_NUM];
 
 static inline long long timeval_to_us(const struct timespec *tv)
 {
@@ -129,7 +131,7 @@ static int do_directio_read(int fd, unsigned int timeout, char *buf, int sz)
 	return 0;
 }
 
-int check_args_valid(int io_num, int base_num)
+int check_args_valid(int io_num, double base_num)
 {
 	if ((io_num < MIN_IO_NUM) || (io_num > MAX_IO_NUM)) {
 		pp_pl_log(0, "args io_num is outside the valid range");
@@ -149,7 +151,7 @@ int check_args_valid(int io_num, int base_num)
  * "io_num=20 base_num=10", this function can get io_num value 20 and
  * base_num value 10.
  */
-static int get_ionum_and_basenum(char *args, int *ionum, int *basenum)
+static int get_ionum_and_basenum(char *args, int *ionum, double *basenum)
 {
 	char split_char[] = " \t";
 	char *arg, *temp;
@@ -177,7 +179,7 @@ static int get_ionum_and_basenum(char *args, int *ionum, int *basenum)
 			flag_io = 1;
 		}
 		else if (!strncmp(str, "base_num=", 9) && strlen(str) > 9) {
-			*basenum = (int)strtol(str + 9, &str_inval, 10);
+			*basenum = strtod(str + 9, &str_inval);
 			if (str == str_inval)
 				goto out;
 			flag_base = 1;
@@ -196,56 +198,36 @@ out:
 	return 0;
 }
 
-long long calc_standard_deviation(long long *path_latency, int size,
-				  long long avglatency)
+double calc_standard_deviation(double *lg_path_latency, int size,
+				  double lg_avglatency)
 {
 	int index;
-	long long total = 0;
+	double sum = 0;
 
 	for (index = 0; index < size; index++) {
-		total +=
-		    (path_latency[index] - avglatency) * (path_latency[index] -
-							  avglatency);
+		sum += (lg_path_latency[index] - lg_avglatency) *
+			(lg_path_latency[index] - lg_avglatency);
 	}
 
-	total /= (size - 1);
+	sum /= (size - 1);
 
-	return (long long)sqrt((double)total);
+	return sqrt(sum);
 }
 
-int calcPrio(double avglatency, double max_avglatency, double min_avglatency,
-	     double base_num)
+/*
+ * Do not scale the prioriy in a certain range such as [0, 1024]
+ * because scaling will eliminate the effect of base_num.
+ */
+int calcPrio(double lg_avglatency, double lg_maxavglatency,
+		double lg_minavglatency)
 {
-	double lavglatency = log(avglatency) / log(base_num);
-	double lmax_avglatency = log(max_avglatency) / log(base_num);
-	double lmin_avglatency = log(min_avglatency) / log(base_num);
+	if (lg_avglatency <= lg_minavglatency)
+		return lg_maxavglatency - lg_minavglatency;
 
-	if (lavglatency <= lmin_avglatency)
-		return (int)(lmax_avglatency + 1.);
-
-	if (lavglatency > lmax_avglatency)
+	if (lg_avglatency >= lg_maxavglatency)
 		return 0;
 
-	return (int)(lmax_avglatency - lavglatency + 1.);
-}
-
-/* Calc the latency interval corresponding to the average latency */
-long long calc_latency_interval(double avglatency, double max_avglatency,
-				double min_avglatency, double base_num)
-{
-	double lavglatency = log(avglatency) / log(base_num);
-	double lmax_avglatency = log(max_avglatency) / log(base_num);
-	double lmin_avglatency = log(min_avglatency) / log(base_num);
-
-	if ((lavglatency <= lmin_avglatency)
-	    || (lavglatency > lmax_avglatency))
-		return 0;	/* Invalid value */
-
-	if ((double)((int)lavglatency) == lavglatency)
-		return (long long)(avglatency - (avglatency / base_num));
-	else
-		return (long long)(pow(base_num, (double)((int)lavglatency + 1))
-				   - pow(base_num, (double)((int)lavglatency)));
+	return lg_maxavglatency - lg_avglatency;
 }
 
 int getprio(struct path *pp, char *args, unsigned int timeout)
@@ -253,26 +235,34 @@ int getprio(struct path *pp, char *args, unsigned int timeout)
 	int rc, temp;
 	int index = 0;
 	int io_num = 0;
-	int base_num = 0;
-	long long avglatency;
-	long long latency_interval;
-	long long standard_deviation;
-	long long toldelay = 0;
+	double base_num = 0;
+	double lg_avglatency, lg_maxavglatency, lg_minavglatency;
+	double standard_deviation;
+	double lg_toldelay = 0;
 	long long before, after;
 	struct timespec tv;
 	int blksize;
 	char *buf;
 	int restore_flags = 0;
+	double lg_base;
+	long long sum_latency = 0;
+	long long arith_mean_lat;
 
 	if (pp->fd < 0)
 		return -1;
 
 	if (get_ionum_and_basenum(args, &io_num, &base_num) == 0) {
-		pp_pl_log(0, "%s: get path_latency args fail", pp->dev);
-		return DEFAULT_PRIORITY;
+		io_num = DEF_IO_NUM;
+		base_num = DEF_BASE_NUM;
+		pp_pl_log(0, "%s: fails to get path_latency args, set default:"
+				"io_num=%d base_num=%.3lf",
+				pp->dev, io_num, base_num);
 	}
 
-	memset(path_latency, 0, sizeof(path_latency));
+	memset(lg_path_latency, 0, sizeof(lg_path_latency));
+	lg_base = log(base_num);
+	lg_maxavglatency = log(MAX_AVG_LATENCY) / lg_base;
+	lg_minavglatency = log(MIN_AVG_LATENCY) / lg_base;
 
 	prepare_directio_read(pp->fd, &blksize, &buf, &restore_flags);
 
@@ -289,43 +279,68 @@ int getprio(struct path *pp, char *args, unsigned int timeout)
 
 		(void)clock_gettime(CLOCK_MONOTONIC, &tv);
 		after = timeval_to_us(&tv);
-
-		path_latency[index] = after - before;
-		toldelay += path_latency[index++];
+		/*
+		 * We assume that the latency complies with Log-normal
+		 * distribution. The logarithm of latency is in normal
+		 * distribution.
+		 */
+		lg_path_latency[index] = log(after - before) / lg_base;
+		lg_toldelay += lg_path_latency[index++];
+		sum_latency += after - before;
 	}
 
 	cleanup_directio_read(pp->fd, buf, restore_flags);
 
-	avglatency = toldelay / (long long)io_num;
-	pp_pl_log(4, "%s: average latency is (%lld us)", pp->dev, avglatency);
+	lg_avglatency = lg_toldelay / (long long)io_num;
+	arith_mean_lat = sum_latency / (long long)io_num;
+	pp_pl_log(4, "%s: arithmetic mean latency is (%lld us), geometric mean latency is (%lld us)",
+			pp->dev, arith_mean_lat,
+			(long long)pow(base_num, lg_avglatency));
 
-	if (avglatency > MAX_AVG_LATENCY) {
+	if (lg_avglatency > lg_maxavglatency) {
 		pp_pl_log(0,
 			  "%s: average latency (%lld us) is outside the thresold (%lld us)",
-			  pp->dev, avglatency, (long long)MAX_AVG_LATENCY);
+			  pp->dev, (long long)pow(base_num, lg_avglatency),
+			  (long long)MAX_AVG_LATENCY);
 		return DEFAULT_PRIORITY;
 	}
 
+	standard_deviation = calc_standard_deviation(lg_path_latency,
+			index, lg_avglatency);
+	/*
+	 * In calPrio(), we let prio y = f(x) = log(max, base) - log (x, base);
+	 * So if we want to let the priority of the latency outside 2 standard
+	 * deviations can be distinguished from the latency inside 2 standard
+	 * deviation, in others words at most 95% are the same and at least 5%
+	 * are different according interval estimation of normal distribution,
+	 * we should warn the user to set the base_num to be smaller if the
+	 * log(x_threshold, base) is small than 2 standard deviation.
+	 * x_threshold is derived from:
+	 * y + 1 = f(x) + 1 = f(x) + log(base, base), so x_threadshold =
+	 * base_num; Note that we only can compare the logarithm of x_threshold
+	 * with the standard deviation because the standard deviation is derived
+	 * from logarithm of latency.
+	 *
+	 * therefore , we recommend the base_num to meet the condition :
+	 * 1 <= 2 * standard_deviation
+	 */
+	pp_pl_log(5, "%s: standard deviation for logarithm of latency = %.6f",
+			pp->dev, standard_deviation);
+	if (standard_deviation <= 0.5)
+		pp_pl_log(3, "%s: the base_num(%.3lf) is too big to distinguish different priority "
+			  "of two far-away latency. It is recommend to be set smaller",
+			  pp->dev, base_num);
 	/*
-	 * Min average latency and max average latency are constant, the args
-	 * base_num set can change latency_interval value corresponding to
-	 * avglatency and is not constant.
-	 * Warn the user if latency_interval is smaller than (2 * standard_deviation),
-	 * or equal.
+	 * If the standard deviation is too large , we should also warn the user
 	 */
-	standard_deviation =
-	    calc_standard_deviation(path_latency, index, avglatency);
-	latency_interval =
-	    calc_latency_interval(avglatency, MAX_AVG_LATENCY, MIN_AVG_LATENCY,
-				  base_num);
-	if ((latency_interval != 0)
-	    && (latency_interval <= (2 * standard_deviation)))
-		pp_pl_log(3,
-			  "%s: latency interval (%lld) according to average latency (%lld us) is smaller than "
-			  "2 * standard deviation (%lld us), or equal, args base_num (%d) needs to be set bigger value",
-			  pp->dev, latency_interval, avglatency,
-			  standard_deviation, base_num);
-
-	rc = calcPrio(avglatency, MAX_AVG_LATENCY, MIN_AVG_LATENCY, base_num);
+
+	if (standard_deviation > 4)
+		pp_pl_log(3, "%s: the base_num(%.3lf) is too small to avoid noise disturbance "
+			  ".It is recommend to be set larger",
+			  pp->dev, base_num);
+
+
+	rc = calcPrio(lg_avglatency, lg_maxavglatency, lg_minavglatency);
+
 	return rc;
 }
-- 
2.11.1

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

* Re: [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency
  2017-09-21 13:23 [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
                   ` (3 preceding siblings ...)
  2017-09-21 13:23 ` [PATCH V2 4/4] multipath-tools: calculate standard deviation on a logarithmic scale for prioritizer path_latency Guan Junxiong
@ 2017-10-10 13:28 ` Guan Junxiong
  4 siblings, 0 replies; 6+ messages in thread
From: Guan Junxiong @ 2017-10-10 13:28 UTC (permalink / raw)
  To: christophe.varoqui, dm-devel, hare
  Cc: chengjike.cheng, xose.vazquez, Yang Feng, shenhong09, niuhaoxin, mwilck

Hi Christophe,

Please consider this series of patches.

>
> This series of patches help to make IO processing more common for
> path-latency prioritizer, make the multipath.conf more user-friendly
> and fixup the error of calculation on log scale standard deviation.
>

Thanks very much.

Regards
Guan

On 2017/9/21 21:23, Guan Junxiong wrote:
> Hi Christophe,
> 
> Please consider this series of patches.
> 
> This series of patches help to make IO processing more common for
> path-latency prioritizer, make the multipath.conf more user-friendly
> and fixup the error of calculation on log scale standard deviation.  
> 
> First, the SCSI-to-NVMe translations which was blamed broken has been removed
> since linux kernel 4.13, so that SG_IO IOCTL used in the reading is not
> supported. Instead, PATCH 1/4 drops sg_read method and uses direct IO
> reading both for NVMe device and SCSI device.
> 
> Second, the original prio_args for prioritizer is like this: 20|10 which
> is somewhat unconvenient for user. PATCH 3/4 drops it and use a syntax that
> similar to other prioritizers, for example :
> "base_num=5 io_num=10".
> 
> PATCH 2/4 can be an independent patch.
> 
> Third, Martin has pointed that standard deviation in the path-latency prioritizer
> is wrong. We have solved this and here come the PATCH 4/4.  
> 
> 
> Thanks
> Guan Junxiong
> 
> 
> Changes from V1:
> * add PATCH 4/4 
> * rebase on the 0.7.3 tag
> 
> Junxiong Guan (3):
>   multipath-tools: use direct IO for path latency prioritizer
>   multipath-tools: move get_next_string to util
>   multipath-tools: use user-friendly prio_args for path-latency
> 
>  libmultipath/prioritizers/path_latency.c | 156 +++++++++++++++++++++++--------
>  libmultipath/prioritizers/weightedpath.c |  10 +-
>  libmultipath/util.c                      |   9 ++
>  libmultipath/util.h                      |   1 +
>  multipath/multipath.conf.5               |   2 +-
>  5 files changed, 127 insertions(+), 51 deletions(-)
> 
> Guan Junxiong (4):
>   multipath-tools: use direct IO for path latency prioritizer
>   multipath-tools: move get_next_string to util
>   multipath-tools: use user-friendly prio_args for path-latency
>   multipath-tools: calculate standard deviation on a logarithmic scale
>     for prioritizer path_latency
> 
>  libmultipath/prioritizers/path_latency.c | 321 ++++++++++++++++++++-----------
>  libmultipath/prioritizers/weightedpath.c |  10 +-
>  libmultipath/util.c                      |   9 +
>  libmultipath/util.h                      |   1 +
>  multipath/multipath.conf.5               |   2 +-
>  5 files changed, 217 insertions(+), 126 deletions(-)
> 

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

end of thread, other threads:[~2017-10-10 13:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21 13:23 [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong
2017-09-21 13:23 ` [PATCH V2 1/4] multipath-tools: use direct IO for path latency prioritizer Guan Junxiong
2017-09-21 13:23 ` [PATCH V2 2/4] multipath-tools: move get_next_string to util Guan Junxiong
2017-09-21 13:23 ` [PATCH V2 3/4] multipath-tools: use user-friendly prio_args for path-latency Guan Junxiong
2017-09-21 13:23 ` [PATCH V2 4/4] multipath-tools: calculate standard deviation on a logarithmic scale for prioritizer path_latency Guan Junxiong
2017-10-10 13:28 ` [PATCH V2 0/4]multipath-tools: some fixup and enhancement for path-latency Guan Junxiong

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