* [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 related [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 related [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 related [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 related [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, other threads:[~2021-04-28 5:00 UTC | newest]
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
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.