Linux-RDMA Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHv3 for-next 0/4] New multipath policy for RTRS client
@ 2021-04-07 11:34 Gioh Kim
  2021-04-07 11:34 ` [PATCHv3 for-next 1/4] RDMA/rtrs-clt: Add a minimum latency multipath policy Gioh Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-07 11:34 UTC (permalink / raw)
  To: linux-rdma
  Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang, Gioh Kim

This patch set introduces new multipath policy 'min-latency'.
The latency is a time calculated by the heart-beat messages. Whenever
the client sends heart-beat message, it checks the time gap between
sending the heart-beat message and receiving the ACK. So this value
can be changed regularly.
If client has multi-path, it can send IO via a path having the least
latency.

V3->V2: use sysfs_emit instead of scnprintf
V2->V1: use sysfs_emit instead of sprintf

Gioh Kim (3):
  RDMA/rtrs-clt: Add a minimum latency multipath policy
  RDMA/rtrs-clt: new sysfs attribute to print the latency of each path
  Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy

Md Haris Iqbal (1):
  RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its
    stats

 .../ABI/testing/sysfs-class-rtrs-client       | 12 ++++
 drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c  | 35 ++++++++++-
 drivers/infiniband/ulp/rtrs/rtrs-clt.c        | 60 ++++++++++++++++++-
 drivers/infiniband/ulp/rtrs/rtrs-clt.h        |  1 +
 drivers/infiniband/ulp/rtrs/rtrs-pri.h        |  2 +
 drivers/infiniband/ulp/rtrs/rtrs.c            |  3 +
 6 files changed, 109 insertions(+), 4 deletions(-)

-- 
2.25.1


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

* [PATCHv3 for-next 1/4] RDMA/rtrs-clt: Add a minimum latency multipath policy
  2021-04-07 11:34 [PATCHv3 for-next 0/4] New multipath policy for RTRS client Gioh Kim
@ 2021-04-07 11:34 ` Gioh Kim
  2021-04-07 11:34 ` [PATCHv3 for-next 2/4] RDMA/rtrs-clt: new sysfs attribute to print the latency of each path Gioh Kim
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-07 11:34 UTC (permalink / raw)
  To: linux-rdma
  Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
	Gioh Kim, Gioh Kim

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

This patch adds new multipath policy: min-latency.
Client checks the latency of each path when it sends the heart-beat.
And it sends IO to the path with the minimum latency.

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c | 19 +++++--
 drivers/infiniband/ulp/rtrs/rtrs-clt.c       | 57 +++++++++++++++++++-
 drivers/infiniband/ulp/rtrs/rtrs-clt.h       |  1 +
 drivers/infiniband/ulp/rtrs/rtrs-pri.h       |  2 +
 drivers/infiniband/ulp/rtrs/rtrs.c           |  3 ++
 5 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
index c502dcbae9bb..c8669d3f79f1 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
@@ -101,6 +101,9 @@ static ssize_t mpath_policy_show(struct device *dev,
 	case MP_POLICY_MIN_INFLIGHT:
 		return sysfs_emit(page, "min-inflight (MI: %d)\n",
 				  clt->mp_policy);
+	case MP_POLICY_MIN_LATENCY:
+		return sysfs_emit(page, "min-latency (ML: %d)\n",
+				  clt->mp_policy);
 	default:
 		return sysfs_emit(page, "Unknown (%d)\n", clt->mp_policy);
 	}
@@ -114,22 +117,32 @@ static ssize_t mpath_policy_store(struct device *dev,
 	struct rtrs_clt *clt;
 	int value;
 	int ret;
+	size_t len = 0;
 
 	clt = container_of(dev, struct rtrs_clt, dev);
 
 	ret = kstrtoint(buf, 10, &value);
 	if (!ret && (value == MP_POLICY_RR ||
-		     value == MP_POLICY_MIN_INFLIGHT)) {
+		     value == MP_POLICY_MIN_INFLIGHT ||
+		     value == MP_POLICY_MIN_LATENCY)) {
 		clt->mp_policy = value;
 		return count;
 	}
 
+	/* distinguish "mi" and "min-latency" with length */
+	len = strnlen(buf, NAME_MAX);
+	if (buf[len - 1] == '\n')
+		len--;
+
 	if (!strncasecmp(buf, "round-robin", 11) ||
-	    !strncasecmp(buf, "rr", 2))
+	    (len == 2 && !strncasecmp(buf, "rr", 2)))
 		clt->mp_policy = MP_POLICY_RR;
 	else if (!strncasecmp(buf, "min-inflight", 12) ||
-		 !strncasecmp(buf, "mi", 2))
+		 (len == 2 && !strncasecmp(buf, "mi", 2)))
 		clt->mp_policy = MP_POLICY_MIN_INFLIGHT;
+	else if (!strncasecmp(buf, "min-latency", 11) ||
+		 (len == 2 && !strncasecmp(buf, "ml", 2)))
+		clt->mp_policy = MP_POLICY_MIN_LATENCY;
 	else
 		return -EINVAL;
 
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 4369e4cf13f0..f3b8151052cb 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -633,6 +633,8 @@ static void rtrs_clt_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
 		} else if (imm_type == RTRS_HB_ACK_IMM) {
 			WARN_ON(con->c.cid);
 			sess->s.hb_missed_cnt = 0;
+			sess->s.hb_cur_latency =
+				ktime_sub(ktime_get(), sess->s.hb_last_sent);
 			if (sess->flags & RTRS_MSG_NEW_RKEY_F)
 				return  rtrs_clt_recv_done(con, wc);
 		} else {
@@ -831,6 +833,57 @@ static struct rtrs_clt_sess *get_next_path_min_inflight(struct path_it *it)
 	return min_path;
 }
 
+/**
+ * get_next_path_min_latency() - Returns path with minimal latency.
+ * @it:	the path pointer
+ *
+ * Return: a path with the lowest latency or NULL if all paths are tried
+ *
+ * Locks:
+ *    rcu_read_lock() must be hold.
+ *
+ * Related to @MP_POLICY_MIN_LATENCY
+ *
+ * This DOES skip an already-tried path.
+ * There is a skip-list to skip a path if the path has tried but failed.
+ * It will try the minimum latency path and then the second minimum latency
+ * path and so on. Finally it will return NULL if all paths are tried.
+ * Therefore the caller MUST check the returned
+ * path is NULL and trigger the IO error.
+ */
+static struct rtrs_clt_sess *get_next_path_min_latency(struct path_it *it)
+{
+	struct rtrs_clt_sess *min_path = NULL;
+	struct rtrs_clt *clt = it->clt;
+	struct rtrs_clt_sess *sess;
+	ktime_t min_latency = INT_MAX;
+	ktime_t latency;
+
+	list_for_each_entry_rcu(sess, &clt->paths_list, s.entry) {
+		if (unlikely(READ_ONCE(sess->state) != RTRS_CLT_CONNECTED))
+			continue;
+
+		if (unlikely(!list_empty(raw_cpu_ptr(sess->mp_skip_entry))))
+			continue;
+
+		latency = sess->s.hb_cur_latency;
+
+		if (latency < min_latency) {
+			min_latency = latency;
+			min_path = sess;
+		}
+	}
+
+	/*
+	 * add the path to the skip list, so that next time we can get
+	 * a different one
+	 */
+	if (min_path)
+		list_add(raw_cpu_ptr(min_path->mp_skip_entry), &it->skip_list);
+
+	return min_path;
+}
+
 static inline void path_it_init(struct path_it *it, struct rtrs_clt *clt)
 {
 	INIT_LIST_HEAD(&it->skip_list);
@@ -839,8 +892,10 @@ static inline void path_it_init(struct path_it *it, struct rtrs_clt *clt)
 
 	if (clt->mp_policy == MP_POLICY_RR)
 		it->next_path = get_next_path_rr;
-	else
+	else if (clt->mp_policy == MP_POLICY_MIN_INFLIGHT)
 		it->next_path = get_next_path_min_inflight;
+	else
+		it->next_path = get_next_path_min_latency;
 }
 
 static inline void path_it_deinit(struct path_it *it)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.h b/drivers/infiniband/ulp/rtrs/rtrs-clt.h
index 59ea2ec44fe5..5c0cea8dd83e 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.h
@@ -30,6 +30,7 @@ enum rtrs_clt_state {
 enum rtrs_mp_policy {
 	MP_POLICY_RR,
 	MP_POLICY_MIN_INFLIGHT,
+	MP_POLICY_MIN_LATENCY,
 };
 
 /* see Documentation/ABI/testing/sysfs-class-rtrs-client for details */
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index 1b31bda9ca78..bcad5e2168c5 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -112,6 +112,8 @@ struct rtrs_sess {
 	unsigned int		hb_interval_ms;
 	unsigned int		hb_missed_cnt;
 	unsigned int		hb_missed_max;
+	ktime_t			hb_last_sent;
+	ktime_t			hb_cur_latency;
 };
 
 /* rtrs information unit */
diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
index bc08b7f6e5e2..a7847282a2eb 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs.c
@@ -337,6 +337,9 @@ static void hb_work(struct work_struct *work)
 		schedule_hb(sess);
 		return;
 	}
+
+	sess->hb_last_sent = ktime_get();
+
 	imm = rtrs_to_imm(RTRS_HB_MSG_IMM, 0);
 	err = rtrs_post_rdma_write_imm_empty(usr_con, sess->hb_cqe, imm,
 					     0, NULL);
-- 
2.25.1


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

* [PATCHv3 for-next 2/4] RDMA/rtrs-clt: new sysfs attribute to print the latency of each path
  2021-04-07 11:34 [PATCHv3 for-next 0/4] New multipath policy for RTRS client Gioh Kim
  2021-04-07 11:34 ` [PATCHv3 for-next 1/4] RDMA/rtrs-clt: Add a minimum latency multipath policy Gioh Kim
@ 2021-04-07 11:34 ` Gioh Kim
  2021-04-07 11:34 ` [PATCHv3 for-next 3/4] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats Gioh Kim
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-07 11:34 UTC (permalink / raw)
  To: linux-rdma
  Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
	Gioh Kim, Gioh Kim

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

It shows the latest latency that the client checked when sending
the heart-beat.

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
index c8669d3f79f1..b083154f567e 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
@@ -355,6 +355,21 @@ static ssize_t rtrs_clt_hca_name_show(struct kobject *kobj,
 static struct kobj_attribute rtrs_clt_hca_name_attr =
 	__ATTR(hca_name, 0444, rtrs_clt_hca_name_show, NULL);
 
+static ssize_t rtrs_clt_cur_latency_show(struct kobject *kobj,
+				    struct kobj_attribute *attr,
+				    char *page)
+{
+	struct rtrs_clt_sess *sess;
+
+	sess = container_of(kobj, struct rtrs_clt_sess, kobj);
+
+	return sysfs_emit(page, "%lld ns\n",
+			  ktime_to_ns(sess->s.hb_cur_latency));
+}
+
+static struct kobj_attribute rtrs_clt_cur_latency_attr =
+	__ATTR(cur_latency, 0444, rtrs_clt_cur_latency_show, NULL);
+
 static ssize_t rtrs_clt_src_addr_show(struct kobject *kobj,
 				       struct kobj_attribute *attr,
 				       char *page)
@@ -398,6 +413,7 @@ static struct attribute *rtrs_clt_sess_attrs[] = {
 	&rtrs_clt_reconnect_attr.attr,
 	&rtrs_clt_disconnect_attr.attr,
 	&rtrs_clt_remove_path_attr.attr,
+	&rtrs_clt_cur_latency_attr.attr,
 	NULL,
 };
 
-- 
2.25.1


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

* [PATCHv3 for-next 3/4] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats
  2021-04-07 11:34 [PATCHv3 for-next 0/4] New multipath policy for RTRS client Gioh Kim
  2021-04-07 11:34 ` [PATCHv3 for-next 1/4] RDMA/rtrs-clt: Add a minimum latency multipath policy Gioh Kim
  2021-04-07 11:34 ` [PATCHv3 for-next 2/4] RDMA/rtrs-clt: new sysfs attribute to print the latency of each path Gioh Kim
@ 2021-04-07 11:34 ` Gioh Kim
  2021-04-07 11:34 ` [PATCHv3 for-next 4/4] Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy Gioh Kim
  2021-04-13 22:47 ` [PATCHv3 for-next 0/4] New multipath policy for RTRS client Jason Gunthorpe
  4 siblings, 0 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-07 11:34 UTC (permalink / raw)
  To: linux-rdma
  Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang,
	Md Haris Iqbal, Gioh Kim

From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>

KASAN detected the following BUG:
[  230.436512] ==================================================================
[  230.437182] BUG: KASAN: use-after-free in get_next_path_min_inflight+0x95/0x150 [rtrs_client]
[  230.437632] Read of size 4 at addr ffff88a796b4bb50 by task fio/4130

[  230.438069] CPU: 32 PID: 4130 Comm: fio Tainted: G           O      5.4.84-pserver #5.4.84-1+feature+linux+5.4.y+dbg+20201216.1319+b6b887b~deb10
[  230.438079] Hardware name: Supermicro H8QG6/H8QG6, BIOS 3.00       09/04/2012
[  230.438088] Call Trace:
[  230.438111]  dump_stack+0x96/0xe0
[  230.438136]  print_address_description.constprop.4+0x1f/0x300
[  230.438150]  ? irq_work_claim+0x2e/0x50
[  230.438172]  __kasan_report.cold.8+0x78/0x92
[  230.438203]  ? get_next_path_min_inflight+0x95/0x150 [rtrs_client]
[  230.438234]  kasan_report+0x10/0x20
[  230.438249]  check_memory_region+0x144/0x1c0
[  230.438274]  get_next_path_min_inflight+0x95/0x150 [rtrs_client]
[  230.438312]  rtrs_clt_request+0x1fe/0x700 [rtrs_client]
[  230.438364]  ? rtrs_clt_close_work+0x40/0x40 [rtrs_client]
[  230.438395]  ? rtrs_clt_change_state_get_old+0x70/0x70 [rtrs_client]
[  230.438417]  ? blk_mq_start_request+0x1a4/0x2c0
[  230.438430]  ? blk_rq_map_sg+0x3d5/0xaa0
[  230.438468]  ? round_jiffies_up+0x60/0x90
[  230.438511]  rnbd_queue_rq+0x3e2/0x870 [rnbd_client]
[  230.438567]  ? rnbd_softirq_done_fn+0x90/0x90 [rnbd_client]
[  230.438587]  ? rnbd_get_permit+0x50/0x50 [rnbd_client]
[  230.438601]  ? __lock_acquire+0x68e/0x23a0
[  230.438635]  ? blk_mq_get_driver_tag+0xbe/0x250
[  230.438652]  ? blk_mq_dequeue_from_ctx+0x4d0/0x4d0
[  230.438663]  ? lock_acquire+0xf3/0x210
[  230.438726]  __blk_mq_try_issue_directly+0x272/0x390
[  230.438752]  ? blk_mq_get_driver_tag+0x250/0x250
[  230.438785]  ? rcu_is_watching+0x34/0x50
[  230.438816]  blk_mq_request_issue_directly+0xa8/0xf0
[  230.438833]  ? blk_mq_flush_plug_list+0x690/0x690
[  230.438859]  ? lock_downgrade+0x390/0x390
[  230.438892]  ? lock_acquire+0xf3/0x210
[  230.438920]  blk_mq_try_issue_list_directly+0xa1/0x160
[  230.438952]  blk_mq_sched_insert_requests+0x23c/0x390
[  230.438992]  blk_mq_flush_plug_list+0x361/0x690
[  230.439037]  ? blk_mq_insert_requests+0x300/0x300
[  230.439058]  ? current_time+0x8c/0xe0
[  230.439074]  ? timestamp_truncate+0x180/0x180
[  230.439101]  ? file_remove_privs+0xb4/0x1f0
[  230.439139]  blk_flush_plug_list+0x1d1/0x210
[  230.439167]  ? blk_insert_cloned_request+0x1e0/0x1e0
[  230.439220]  blk_finish_plug+0x3c/0x54
[  230.439243]  blkdev_write_iter+0x173/0x260
[  230.439272]  ? bd_finish_claiming+0xe0/0xe0
[  230.439298]  ? 0xffffffff9a000000
[  230.439330]  ? rw_verify_area+0xd9/0x130
[  230.439359]  aio_write+0x1d3/0x300
[  230.439387]  ? aio_read+0x260/0x260
[  230.439477]  ? lock_downgrade+0x390/0x390
[  230.439497]  ? lock_acquire+0xf3/0x210
[  230.439512]  ? __might_fault+0x7d/0xe0
[  230.439570]  io_submit_one+0xccc/0x1920
[  230.439633]  ? aio_poll_complete_work+0x850/0x850
[  230.439735]  ? __x64_sys_io_submit+0x118/0x380
[  230.439748]  __x64_sys_io_submit+0x118/0x380
[  230.439777]  ? __ia32_compat_sys_io_submit+0x360/0x360
[  230.439793]  ? __x64_sys_io_getevents+0xd7/0x150
[  230.439807]  ? mark_held_locks+0x29/0xa0
[  230.439827]  ? entry_SYSCALL_64_after_hwframe+0x3e/0xbe
[  230.439840]  ? trace_hardirqs_off_caller+0x15/0x110
[  230.439857]  ? mark_held_locks+0x29/0xa0
[  230.439893]  ? do_syscall_64+0x68/0x270
[  230.439903]  do_syscall_64+0x68/0x270
[  230.439924]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  230.439936] RIP: 0033:0x7f8f10233f59
[  230.439948] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
[  230.439958] RSP: 002b:00007fff1df1d238 EFLAGS: 00000246 ORIG_RAX: 00000000000000d1
[  230.439970] RAX: ffffffffffffffda RBX: 00007f8f104ec360 RCX: 00007f8f10233f59
[  230.439980] RDX: 0000555c4c1f9440 RSI: 0000000000000001 RDI: 00007f8f04730000
[  230.439989] RBP: 00007f8f04730000 R08: 0000555c4c1c97f0 R09: 00000000000001e0
[  230.439998] R10: 0000555c4c1f9670 R11: 0000000000000246 R12: 0000000000000001
[  230.440007] R13: 0000000000000000 R14: 0000555c4c1f9440 R15: 00007f8ee30b5210

[  230.440257] Allocated by task 3440:
[  230.440471]  save_stack+0x19/0x80
[  230.440482]  __kasan_kmalloc.constprop.9+0xc1/0xd0
[  230.440492]  kmem_cache_alloc_trace+0x15b/0x350
[  230.440508]  alloc_sess+0xf4/0x570 [rtrs_client]
[  230.440524]  rtrs_clt_open+0x3b4/0x780 [rtrs_client]
[  230.440538]  find_and_get_or_create_sess+0x649/0x9d0 [rnbd_client]
[  230.440551]  rnbd_clt_map_device+0xd7/0xf50 [rnbd_client]
[  230.440565]  rnbd_clt_map_device_store+0x4ee/0x970 [rnbd_client]
[  230.440577]  kernfs_fop_write+0x141/0x240
[  230.440587]  vfs_write+0xf3/0x280
[  230.440598]  ksys_write+0xba/0x150
[  230.440608]  do_syscall_64+0x68/0x270
[  230.440619]  entry_SYSCALL_64_after_hwframe+0x49/0xbe

[  230.440806] Freed by task 4148:
[  230.441013]  save_stack+0x19/0x80
[  230.441024]  __kasan_slab_free+0x125/0x170
[  230.441034]  kfree+0xe7/0x3f0
[  230.441045]  kobject_put+0xd3/0x240
[  230.441061]  rtrs_clt_destroy_sess_files+0x3f/0x60 [rtrs_client]
[  230.441076]  rtrs_clt_remove_path_from_sysfs+0x95/0xe0 [rtrs_client]
[  230.441092]  rtrs_clt_remove_path_store+0x3e/0xa0 [rtrs_client]
[  230.441103]  kernfs_fop_write+0x141/0x240
[  230.441113]  vfs_write+0xf3/0x280
[  230.441123]  ksys_write+0xba/0x150
[  230.441133]  do_syscall_64+0x68/0x270
[  230.441145]  entry_SYSCALL_64_after_hwframe+0x49/0xbe

[  230.441333] The buggy address belongs to the object at ffff88a796b4bb00
                which belongs to the cache kmalloc-96 of size 96
[  230.441705] The buggy address is located 80 bytes inside of
                96-byte region [ffff88a796b4bb00, ffff88a796b4bb60)
[  230.442063] The buggy address belongs to the page:
[  230.442294] page:ffffea009e5ad2c0 refcount:1 mapcount:0 mapping:ffff8887c6016e00 index:0x0
[  230.442305] flags: 0x12ffff8000000200(slab)
[  230.442320] raw: 12ffff8000000200 dead000000000100 dead000000000122 ffff8887c6016e00
[  230.442332] raw: 0000000000000000 0000000000200020 00000001ffffffff 0000000000000000
[  230.442340] page dumped because: kasan: bad access detected

[  230.442525] Memory state around the buggy address:
[  230.442756]  ffff88a796b4ba00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
[  230.443059]  ffff88a796b4ba80: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
[  230.443359] >ffff88a796b4bb00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
[  230.443681]                                                  ^
[  230.443935]  ffff88a796b4bb80: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
[  230.444233]  ffff88a796b4bc00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
[  230.444529] ==================================================================

When get_next_path_min_inflight is called to select the next path, it
iterates over the list of available rtrs_clt_sess (paths). It then reads
the number of inflight IOs for that path to select one which has the least

But it may so happen that that rtrs_clt_sess (path) is no longer in the
connected state, and like in the above BUG its resources have also been
freed.

So, check the state of the rtrs_clt_sess (path) before going ahead to read
its inflight stats.

Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index f3b8151052cb..96029d4ec26f 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -812,6 +812,9 @@ static struct rtrs_clt_sess *get_next_path_min_inflight(struct path_it *it)
 	int inflight;
 
 	list_for_each_entry_rcu(sess, &clt->paths_list, s.entry) {
+		if (unlikely(READ_ONCE(sess->state) != RTRS_CLT_CONNECTED))
+			continue;
+
 		if (unlikely(!list_empty(raw_cpu_ptr(sess->mp_skip_entry))))
 			continue;
 
-- 
2.25.1


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

* [PATCHv3 for-next 4/4] Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy
  2021-04-07 11:34 [PATCHv3 for-next 0/4] New multipath policy for RTRS client Gioh Kim
                   ` (2 preceding siblings ...)
  2021-04-07 11:34 ` [PATCHv3 for-next 3/4] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats Gioh Kim
@ 2021-04-07 11:34 ` Gioh Kim
  2021-04-13 22:47 ` [PATCHv3 for-next 0/4] New multipath policy for RTRS client Jason Gunthorpe
  4 siblings, 0 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-07 11:34 UTC (permalink / raw)
  To: linux-rdma
  Cc: bvanassche, leon, dledford, jgg, haris.iqbal, jinpu.wang, Gioh Kim

describe new multipath policy min-latency of the RTRS client.

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 Documentation/ABI/testing/sysfs-class-rtrs-client | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-rtrs-client b/Documentation/ABI/testing/sysfs-class-rtrs-client
index 0f7165aab251..49a4157c7bf1 100644
--- a/Documentation/ABI/testing/sysfs-class-rtrs-client
+++ b/Documentation/ABI/testing/sysfs-class-rtrs-client
@@ -34,6 +34,9 @@ Description:	Multipath policy specifies which path should be selected on each IO
 		min-inflight (1):
 		    select path with minimum inflights.
 
+		min-latency (2):
+		    select path with minimum latency.
+
 What:		/sys/class/rtrs-client/<session-name>/paths/
 Date:		Feb 2020
 KernelVersion:	5.7
@@ -95,6 +98,15 @@ KernelVersion:	5.7
 Contact:	Jack Wang <jinpu.wang@cloud.ionos.com> Danil Kipnis <danil.kipnis@cloud.ionos.com>
 Description:	RO, Contains the destination address of the path
 
+What:		/sys/class/rtrs-client/<session-name>/paths/<src@dst>/cur_latency
+Date:		Feb 2020
+KernelVersion:	5.7
+Contact:	Jack Wang <jinpu.wang@cloud.ionos.com> Danil Kipnis <danil.kipnis@cloud.ionos.com>
+Description:	RO, Contains the latency time calculated by the heart-beat messages.
+		Whenever the client sends heart-beat message, it checks the time gap
+		between sending the heart-beat message and receiving the ACK.
+		This value can be changed regularly.
+
 What:		/sys/class/rtrs-client/<session-name>/paths/<src@dst>/stats/reset_all
 Date:		Feb 2020
 KernelVersion:	5.7
-- 
2.25.1


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

* Re: [PATCHv3 for-next 0/4] New multipath policy for RTRS client
  2021-04-07 11:34 [PATCHv3 for-next 0/4] New multipath policy for RTRS client Gioh Kim
                   ` (3 preceding siblings ...)
  2021-04-07 11:34 ` [PATCHv3 for-next 4/4] Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy Gioh Kim
@ 2021-04-13 22:47 ` Jason Gunthorpe
  2021-04-14  4:59   ` Gioh Kim
  4 siblings, 1 reply; 9+ messages in thread
From: Jason Gunthorpe @ 2021-04-13 22:47 UTC (permalink / raw)
  To: Gioh Kim; +Cc: linux-rdma, bvanassche, leon, dledford, haris.iqbal, jinpu.wang

On Wed, Apr 07, 2021 at 01:34:40PM +0200, Gioh Kim wrote:
> This patch set introduces new multipath policy 'min-latency'.
> The latency is a time calculated by the heart-beat messages. Whenever
> the client sends heart-beat message, it checks the time gap between
> sending the heart-beat message and receiving the ACK. So this value
> can be changed regularly.
> If client has multi-path, it can send IO via a path having the least
> latency.
> 
> V3->V2: use sysfs_emit instead of scnprintf
> V2->V1: use sysfs_emit instead of sprintf
> 
> Gioh Kim (3):
>   RDMA/rtrs-clt: Add a minimum latency multipath policy
>   RDMA/rtrs-clt: new sysfs attribute to print the latency of each path
>   Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy

Applied to for-next, thanks

Please be mindful about the subjects, rdma subject start with a
capital letter after the tag

> Md Haris Iqbal (1):
>   RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its
>     stats

This one was replaced by that other patch

Jason

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

* Re: [PATCHv3 for-next 0/4] New multipath policy for RTRS client
  2021-04-13 22:47 ` [PATCHv3 for-next 0/4] New multipath policy for RTRS client Jason Gunthorpe
@ 2021-04-14  4:59   ` Gioh Kim
  2021-04-27 18:59     ` Jason Gunthorpe
  0 siblings, 1 reply; 9+ messages in thread
From: Gioh Kim @ 2021-04-14  4:59 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-rdma, Bart Van Assche, Leon Romanovsky, Doug Ledford,
	Haris Iqbal, Jinpu Wang

On Wed, Apr 14, 2021 at 12:47 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Apr 07, 2021 at 01:34:40PM +0200, Gioh Kim wrote:
> > This patch set introduces new multipath policy 'min-latency'.
> > The latency is a time calculated by the heart-beat messages. Whenever
> > the client sends heart-beat message, it checks the time gap between
> > sending the heart-beat message and receiving the ACK. So this value
> > can be changed regularly.
> > If client has multi-path, it can send IO via a path having the least
> > latency.
> >
> > V3->V2: use sysfs_emit instead of scnprintf
> > V2->V1: use sysfs_emit instead of sprintf
> >
> > Gioh Kim (3):
> >   RDMA/rtrs-clt: Add a minimum latency multipath policy
> >   RDMA/rtrs-clt: new sysfs attribute to print the latency of each path
> >   Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy
>
> Applied to for-next, thanks
>
> Please be mindful about the subjects, rdma subject start with a
> capital letter after the tag
>
> > Md Haris Iqbal (1):
> >   RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its
> >     stats
>
> This one was replaced by that other patch

Hi Jason,

"RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats"
This patch is still necessary. Please apply that patch.

As I wrote in the description of patch
"RDMA/rtrs-clt: destroy sysfs after removing session from active list",
each function still should check the session status because closing
or error recovery paths can change the status.

For example, the client sends the heart-beat and does not get the
response, it changes the session status and stops IO processing.
It is ok if the status is changed after checking status because
the error recovery path does not free memory and only tries to
reconnection.

And closing the session changes the session status and flush all IO,
and then free memory.

Thank you for the review.

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

* Re: [PATCHv3 for-next 0/4] New multipath policy for RTRS client
  2021-04-14  4:59   ` Gioh Kim
@ 2021-04-27 18:59     ` Jason Gunthorpe
  2021-04-28  4:59       ` Gioh Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Gunthorpe @ 2021-04-27 18:59 UTC (permalink / raw)
  To: Gioh Kim
  Cc: linux-rdma, Bart Van Assche, Leon Romanovsky, Doug Ledford,
	Haris Iqbal, Jinpu Wang

On Wed, Apr 14, 2021 at 06:59:59AM +0200, Gioh Kim wrote:
> On Wed, Apr 14, 2021 at 12:47 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Wed, Apr 07, 2021 at 01:34:40PM +0200, Gioh Kim wrote:
> > > This patch set introduces new multipath policy 'min-latency'.
> > > The latency is a time calculated by the heart-beat messages. Whenever
> > > the client sends heart-beat message, it checks the time gap between
> > > sending the heart-beat message and receiving the ACK. So this value
> > > can be changed regularly.
> > > If client has multi-path, it can send IO via a path having the least
> > > latency.
> > >
> > > V3->V2: use sysfs_emit instead of scnprintf
> > > V2->V1: use sysfs_emit instead of sprintf
> > >
> > > Gioh Kim (3):
> > >   RDMA/rtrs-clt: Add a minimum latency multipath policy
> > >   RDMA/rtrs-clt: new sysfs attribute to print the latency of each path
> > >   Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy
> >
> > Applied to for-next, thanks
> >
> > Please be mindful about the subjects, rdma subject start with a
> > capital letter after the tag
> >
> > > Md Haris Iqbal (1):
> > >   RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its
> > >     stats
> >
> > This one was replaced by that other patch
> 
> Hi Jason,
> 
> "RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats"
> This patch is still necessary. Please apply that patch.
> 
> As I wrote in the description of patch
> "RDMA/rtrs-clt: destroy sysfs after removing session from active list",
> each function still should check the session status because closing
> or error recovery paths can change the status.
> 
> For example, the client sends the heart-beat and does not get the
> response, it changes the session status and stops IO processing.
> It is ok if the status is changed after checking status because
> the error recovery path does not free memory and only tries to
> reconnection.
> 
> And closing the session changes the session status and flush all IO,
> and then free memory.

You need to resend it with out the oops message, a patch like this
cannot be correctly fixing an oops like it presents. Confirm you do
not have the oops now that the other patch is merged.

Please explain carefully what it is trying to do

Jason

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

* Re: [PATCHv3 for-next 0/4] New multipath policy for RTRS client
  2021-04-27 18:59     ` Jason Gunthorpe
@ 2021-04-28  4:59       ` Gioh Kim
  0 siblings, 0 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-28  4:59 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-rdma, Bart Van Assche, Leon Romanovsky, Doug Ledford,
	Haris Iqbal, Jinpu Wang

On Tue, Apr 27, 2021 at 9:00 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Wed, Apr 14, 2021 at 06:59:59AM +0200, Gioh Kim wrote:
> > On Wed, Apr 14, 2021 at 12:47 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> > >
> > > On Wed, Apr 07, 2021 at 01:34:40PM +0200, Gioh Kim wrote:
> > > > This patch set introduces new multipath policy 'min-latency'.
> > > > The latency is a time calculated by the heart-beat messages. Whenever
> > > > the client sends heart-beat message, it checks the time gap between
> > > > sending the heart-beat message and receiving the ACK. So this value
> > > > can be changed regularly.
> > > > If client has multi-path, it can send IO via a path having the least
> > > > latency.
> > > >
> > > > V3->V2: use sysfs_emit instead of scnprintf
> > > > V2->V1: use sysfs_emit instead of sprintf
> > > >
> > > > Gioh Kim (3):
> > > >   RDMA/rtrs-clt: Add a minimum latency multipath policy
> > > >   RDMA/rtrs-clt: new sysfs attribute to print the latency of each path
> > > >   Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy
> > >
> > > Applied to for-next, thanks
> > >
> > > Please be mindful about the subjects, rdma subject start with a
> > > capital letter after the tag
> > >
> > > > Md Haris Iqbal (1):
> > > >   RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its
> > > >     stats
> > >
> > > This one was replaced by that other patch
> >
> > Hi Jason,
> >
> > "RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats"
> > This patch is still necessary. Please apply that patch.
> >
> > As I wrote in the description of patch
> > "RDMA/rtrs-clt: destroy sysfs after removing session from active list",
> > each function still should check the session status because closing
> > or error recovery paths can change the status.
> >
> > For example, the client sends the heart-beat and does not get the
> > response, it changes the session status and stops IO processing.
> > It is ok if the status is changed after checking status because
> > the error recovery path does not free memory and only tries to
> > reconnection.
> >
> > And closing the session changes the session status and flush all IO,
> > and then free memory.
>
> You need to resend it with out the oops message, a patch like this
> cannot be correctly fixing an oops like it presents. Confirm you do
> not have the oops now that the other patch is merged.

Yes, that oops is prevented by another patch.
I will change the commit message and send it to you.

>
> Please explain carefully what it is trying to do

Ok, I will.


>
> Jason

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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 11:34 [PATCHv3 for-next 0/4] New multipath policy for RTRS client Gioh Kim
2021-04-07 11:34 ` [PATCHv3 for-next 1/4] RDMA/rtrs-clt: Add a minimum latency multipath policy Gioh Kim
2021-04-07 11:34 ` [PATCHv3 for-next 2/4] RDMA/rtrs-clt: new sysfs attribute to print the latency of each path Gioh Kim
2021-04-07 11:34 ` [PATCHv3 for-next 3/4] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats Gioh Kim
2021-04-07 11:34 ` [PATCHv3 for-next 4/4] Documentation/ABI/rtrs-clt: Add descriptions for min-latency policy Gioh Kim
2021-04-13 22:47 ` [PATCHv3 for-next 0/4] New multipath policy for RTRS client Jason Gunthorpe
2021-04-14  4:59   ` Gioh Kim
2021-04-27 18:59     ` Jason Gunthorpe
2021-04-28  4:59       ` Gioh Kim

Linux-RDMA Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rdma/0 linux-rdma/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rdma linux-rdma/ https://lore.kernel.org/linux-rdma \
		linux-rdma@vger.kernel.org
	public-inbox-index linux-rdma

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rdma


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git