From: Håkon Bugge Date: Wed, 23 May 2018 09:21:08 +0000 (+0200) Subject: Revert "Revert "net/rds: Revert "RDS: add reconnect retry scheme for stalled" X-Git-Tag: v4.1.12-124.31.3~718 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=a5d7143971eda0a7920ec182e81d6aef01b9232a;p=users%2Fjedix%2Flinux-maple.git Revert "Revert "net/rds: Revert "RDS: add reconnect retry scheme for stalled" This reverts commit fde8284e499b ("Revert "net/rds: Revert "RDS: add reconnect retry scheme for stalled connections"""). Here is the commit message for the first revert: Commit 54312602fb26 ("RDS: add reconnect retry scheme for stalled connections") introduces the rds_reconnect_timeout to retry the connection establishment after sysctl_reconnect_retry_ms (default is 1000ms). Nevertheless, this proactive mechanism is overkilled and it is causing long brownout time in the virtualized environment. In short, below are the justifications why said commit is reverted. a) The retry counter starts ticking after RDS received an ADDR_CHANGE event. After receiving an ADDR_CHANGE event, RDS needs to perform shutdown via shutdown_worker. Then, initiate a new connection via connect_worker. Eventually, a CM REQ is only sent out after RDS received RDMA_CM_EVENT_ADDR_RESOLVED and RDMA_CM_EVENT_ROUTE_RESOLVED events. If the retry is made to cater for stalled connection due to missing CM messages, the retry should only happen after a CM REQ is sent. With the current retry scheme (and with the default 1000 ms) that happens after ADDR_CHANGE event, it introduces congestion in the single threaded workqueue. b) Assuming that we modify the retry counter to start ticking after a CM REQ message is sent out. By introducing another retry timeout, it complicates the system tuning. Why? First, the sysctl_reconnect_retry_ms relies on the underlying cma_response_timeout. Any modication of cma_response_timeout requires to tune sysctl_reconnect_retry_ms. Second, it is hard to find a universal timing that fits all configurations (bare-metal, virtualized, mixed environment, and homo/heterogenous system). Signed-off-by: Brian Maly Conflicts: net/rds/ib_cm.c net/rds/rdma_transport.c net/rds/threads.c Fixed checkpatch warnings. Orabug: 28068627 Signed-off-by: Håkon Bugge Reviewed-by: Shannon Nelson Reviewed-by: Zhu Yanjun Signed-off-by: Brian Maly --- diff --git a/net/rds/connection.c b/net/rds/connection.c index cd0817022abd..e6417d812293 100644 --- a/net/rds/connection.c +++ b/net/rds/connection.c @@ -268,9 +268,6 @@ static struct rds_connection *__rds_conn_create(struct net *net, __rds_conn_path_init(conn, cp, is_outgoing); cp->cp_index = i; - cp->cp_reconnect_retry = rds_sysctl_reconnect_retry_ms; - cp->cp_reconnect_retry_count = 0; - if (conn->c_loopback) cp->cp_wq = rds_local_wq; else diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c index ff08c1a17a9e..d1e44efa0d82 100644 --- a/net/rds/ib_cm.c +++ b/net/rds/ib_cm.c @@ -1071,23 +1071,20 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id, rds_ib_stats_inc(s_ib_listen_closed_stale); } else if (rds_conn_state(conn) == RDS_CONN_CONNECTING) { unsigned long now = get_seconds(); - unsigned long retry = conn->c_reconnect_retry; - - /* after retry seconds, give up on - * existing connection attempts and try again. - * At this point it's no longer backoff race but - * something has gone horribly wrong. + /* After 15 seconds, give up on existing connection + * attempts and make them try again. At this point + * it's no longer a race but something has gone + * horribly wrong */ - retry = DIV_ROUND_UP(retry, 1000); if (now > conn->c_connection_start && - now - conn->c_connection_start > retry) { - pr_info("RDS/IB: conn <%pI6c,%pI6c,%d> racing for more than %lus, retry\n", - &conn->c_laddr, &conn->c_faddr, - conn->c_tos, retry); - set_bit(RDS_RECONNECT_TIMEDOUT, - &conn->c_reconn_flags); - rds_conn_drop(conn, DR_RECONNECT_TIMEOUT); + now - conn->c_connection_start > 15) { + rds_rtd_ptr(RDS_RTD_CM, + "RDS/IB: connection <%pI6c,%pI6c,%d> racing for 15s, forcing reset", + &conn->c_laddr, + &conn->c_faddr, + conn->c_tos); + rds_conn_drop(conn, DR_IB_REQ_WHILE_CONNECTING); rds_ib_stats_inc(s_ib_listen_closed_stale); } else { /* Wait and see - our connect may still be succeeding */ diff --git a/net/rds/rdma_transport.c b/net/rds/rdma_transport.c index 80f6a05517d7..ebf336b3e4ae 100644 --- a/net/rds/rdma_transport.c +++ b/net/rds/rdma_transport.c @@ -278,12 +278,8 @@ int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id, "ADDR_CHANGE: calling rds_conn_drop <%pI6c,%pI6c,%d>\n", &conn->c_laddr, &conn->c_faddr, conn->c_tos); - if (!rds_conn_self_loopback_passive(conn)) { - queue_delayed_work(conn->c_path[0].cp_wq, - &conn->c_reconn_w, - msecs_to_jiffies(conn->c_reconnect_retry)); + if (!rds_conn_self_loopback_passive(conn)) rds_conn_drop(conn, DR_IB_ADDR_CHANGE); - } } break; diff --git a/net/rds/rds.h b/net/rds/rds.h index b6011ce79e50..3be84f04df83 100644 --- a/net/rds/rds.h +++ b/net/rds/rds.h @@ -149,8 +149,6 @@ enum { #define RDS_MPATH_WORKERS 8 #define RDS_MPATH_HASH(rs, n) (jhash_1word((rs)->rs_bound_port, \ (rs)->rs_hash_initval) & ((n) - 1)) -/* Bits for c_reconn_flags */ -#define RDS_RECONNECT_TIMEDOUT 0 enum rds_conn_drop_src { /* rds-core */ DR_DEFAULT, @@ -319,6 +317,15 @@ struct rds_connection { unsigned int c_version; struct net *c_net; + /* Re-connect stall diagnostics */ + unsigned long c_reconnect_start; + unsigned int c_reconnect_drops; + int c_reconnect_warn; + int c_reconnect_err; + int c_to_index; + + unsigned int c_reconnect; + /* Qos support */ u8 c_tos; @@ -1139,8 +1146,6 @@ extern unsigned long rds_sysctl_trace_flags; extern unsigned int rds_sysctl_trace_level; extern unsigned int rds_sysctl_shutdown_trace_start_time; extern unsigned int rds_sysctl_shutdown_trace_end_time; -extern unsigned long rds_sysctl_reconnect_retry_ms; -extern unsigned int rds_sysctl_reconnect_max_retries; /* threads.c */ int rds_threads_init(void); diff --git a/net/rds/rds_single_path.h b/net/rds/rds_single_path.h index 4b6f6398eb88..b9051724feff 100644 --- a/net/rds/rds_single_path.h +++ b/net/rds/rds_single_path.h @@ -21,7 +21,6 @@ #define c_send_w c_path[0].cp_send_w #define c_recv_w c_path[0].cp_recv_w #define c_conn_w c_path[0].cp_conn_w -#define c_reconn_w c_path[0].cp_reconn_w #define c_down_w c_path[0].cp_down_w #define c_cm_lock c_path[0].cp_cm_lock #define c_waitq c_path[0].cp_waitq @@ -32,8 +31,6 @@ #define c_acl_init c_path[0].cp_acl_init #define c_connection_start c_path[0].cp_connection_start #define c_reconnect_racing c_path[0].cp_reconnect_racing -#define c_reconnect_retry c_path[0].cp_reconnect_retry -#define c_reconn_flags c_path[0].cp_reconn_flags #define c_reconnect c_path[0].cp_reconnect #define c_to_index c_path[0].cp_to_index #define c_acl_en c_path[0].cp_acl_en diff --git a/net/rds/sysctl.c b/net/rds/sysctl.c index 64b4e77f7855..b22e8b8b6b89 100644 --- a/net/rds/sysctl.c +++ b/net/rds/sysctl.c @@ -52,13 +52,6 @@ unsigned int rds_sysctl_ping_enable = 1; unsigned int rds_sysctl_shutdown_trace_start_time; unsigned int rds_sysctl_shutdown_trace_end_time; -unsigned long rds_sysctl_reconnect_retry_ms = 1000; -static unsigned long reconnect_retry_ms_min = 100; -static unsigned long reconnect_retry_ms_max = 15000; - -unsigned int rds_sysctl_reconnect_max_retries = 60; -static unsigned long reconnect_min_retries = 15; - /* * We have official values, but must maintain the sysctl interface for existing * software that expects to find these values here. @@ -133,25 +126,6 @@ static struct ctl_table rds_sysctl_rds_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = &proc_dointvec, - - }, - { - .procname = "reconnect_retry_ms", - .data = &rds_sysctl_reconnect_retry_ms, - .maxlen = sizeof(unsigned long), - .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = &reconnect_retry_ms_min, - .extra2 = &reconnect_retry_ms_max, - }, - { - .procname = "reconnect_max_retries", - .data = &rds_sysctl_reconnect_max_retries, - .maxlen = sizeof(unsigned int), - .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = &reconnect_min_retries, - .extra2 = &rds_sysctl_reconnect_max_retries, }, { } }; diff --git a/net/rds/threads.c b/net/rds/threads.c index 854c2a0900a3..bd1e64a71fc4 100644 --- a/net/rds/threads.c +++ b/net/rds/threads.c @@ -94,8 +94,6 @@ void rds_connect_path_complete(struct rds_conn_path *cp, int curr) conn, &conn->c_laddr, &conn->c_faddr, conn->c_tos); cp->cp_reconnect_jiffies = 0; - cp->cp_reconnect_retry = rds_sysctl_reconnect_retry_ms; - cp->cp_reconnect_retry_count = 0; set_bit(0, &conn->c_map_queued); queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 0); queue_delayed_work(cp->cp_wq, &cp->cp_recv_w, 0); @@ -148,8 +146,7 @@ void rds_queue_reconnect(struct rds_conn_path *cp) return; set_bit(RDS_RECONNECT_PENDING, &cp->cp_flags); - if (cp->cp_reconnect_jiffies == 0 || - test_and_clear_bit(RDS_RECONNECT_TIMEDOUT, &cp->cp_reconn_flags)) { + if (cp->cp_reconnect_jiffies == 0) { cp->cp_reconnect_jiffies = rds_sysctl_reconnect_min_jiffies; queue_delayed_work(cp->cp_wq, &cp->cp_conn_w, 0); return; @@ -306,28 +303,12 @@ void rds_reconnect_timeout(struct work_struct *work) cp_reconn_w.work); struct rds_connection *conn = cp->cp_conn; - if (cp->cp_reconnect_retry_count > rds_sysctl_reconnect_max_retries) { - pr_info("RDS: connection <%pI6c,%pI6c,%d> reconnect retries(%d) exceeded, stop retry\n", - &conn->c_laddr, &conn->c_faddr, conn->c_tos, - cp->cp_reconnect_retry_count); - return; - } - - if (!rds_conn_up(conn)) { - if (rds_conn_state(conn) == RDS_CONN_DISCONNECTING) { - queue_delayed_work(cp->cp_wq, &cp->cp_reconn_w, - msecs_to_jiffies(100)); - } else { - cp->cp_reconnect_retry_count++; - rds_rtd_ptr(RDS_RTD_CM, - "conn <%pI6c,%pI6c,%d> not up, retry(%d)\n", - &conn->c_laddr, &conn->c_faddr, conn->c_tos, - cp->cp_reconnect_retry_count); - queue_delayed_work(cp->cp_wq, &cp->cp_reconn_w, - msecs_to_jiffies(cp->cp_reconnect_retry)); - set_bit(RDS_RECONNECT_TIMEDOUT, &cp->cp_reconn_flags); - rds_conn_drop(conn, DR_RECONNECT_TIMEOUT); - } + if (!rds_conn_path_up(cp)) { + rds_rtd_ptr(RDS_RTD_CM, + "conn <%pI6c,%pI6c,%d> not up, retry(%d)\n", + &conn->c_laddr, &conn->c_faddr, conn->c_tos, + cp->cp_reconnect_retry_count); + rds_conn_path_drop(cp, DR_RECONNECT_TIMEOUT); } }