Message ID | 20200626183438.20188-1-rao.shoaib@oracle.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [v1] rds: If one path needs re-connection, check all and re-connect | expand |
From: rao.shoaib@oracle.com Date: Fri, 26 Jun 2020 11:34:38 -0700 > +/* Check connectivity of all paths > + */ > +void rds_check_all_paths(struct rds_connection *conn) > +{ > + int i = 0; > + > + do { > + rds_conn_path_connect_if_down(&conn->c_path[i]); > + } while (++i < conn->c_npaths); > +} Please code this loop in a more canonial way: int i; for (i = 0; i < conn->c_npaths, i++) rds_conn_path_connect_if_down(&conn->c_path[i]); Thank you.
On 6/26/20 4:31 PM, David Miller wrote: > From: rao.shoaib@oracle.com > Date: Fri, 26 Jun 2020 11:34:38 -0700 > >> +/* Check connectivity of all paths >> + */ >> +void rds_check_all_paths(struct rds_connection *conn) >> +{ >> + int i = 0; >> + >> + do { >> + rds_conn_path_connect_if_down(&conn->c_path[i]); >> + } while (++i < conn->c_npaths); >> +} > Please code this loop in a more canonial way: > > int i; > > for (i = 0; i < conn->c_npaths, i++) > rds_conn_path_connect_if_down(&conn->c_path[i]); > > Thank you. This was coded in this unusual way because the code is agnostic to the underlying transport. Unfortunately, IB transport does not initialize/use this field where as TCP does and counts starting from one. If this is not acceptable, I would have to introduce a check for the transport or deal with zero count separately. Let me know. Regards, Shoaib
From: Rao Shoaib <rao.shoaib@oracle.com> Date: Mon, 29 Jun 2020 10:55:28 -0700 > This was coded in this unusual way because the code is agnostic to the > underlying transport. Unfortunately, IB transport does not > initialize/use this field where as TCP does and counts starting from > one. Ok, please resubmit, I didn't understand that the conn->c_npaths could be zero. Thank you.
diff --git a/net/rds/connection.c b/net/rds/connection.c index ed7f2133acc2..f2fcab182095 100644 --- a/net/rds/connection.c +++ b/net/rds/connection.c @@ -905,6 +905,17 @@ void rds_conn_path_connect_if_down(struct rds_conn_path *cp) } EXPORT_SYMBOL_GPL(rds_conn_path_connect_if_down); +/* Check connectivity of all paths + */ +void rds_check_all_paths(struct rds_connection *conn) +{ + int i = 0; + + do { + rds_conn_path_connect_if_down(&conn->c_path[i]); + } while (++i < conn->c_npaths); +} + void rds_conn_connect_if_down(struct rds_connection *conn) { WARN_ON(conn->c_trans->t_mp_capable); diff --git a/net/rds/rds.h b/net/rds/rds.h index 6019b0c004a9..106e862996b9 100644 --- a/net/rds/rds.h +++ b/net/rds/rds.h @@ -778,6 +778,7 @@ void rds_conn_drop(struct rds_connection *conn); void rds_conn_path_drop(struct rds_conn_path *cpath, bool destroy); void rds_conn_connect_if_down(struct rds_connection *conn); void rds_conn_path_connect_if_down(struct rds_conn_path *cp); +void rds_check_all_paths(struct rds_connection *conn); void rds_for_each_conn_info(struct socket *sock, unsigned int len, struct rds_info_iterator *iter, struct rds_info_lengths *lens, @@ -822,6 +823,12 @@ rds_conn_path_up(struct rds_conn_path *cp) return atomic_read(&cp->cp_state) == RDS_CONN_UP; } +static inline int +rds_conn_path_down(struct rds_conn_path *cp) +{ + return atomic_read(&cp->cp_state) == RDS_CONN_DOWN; +} + static inline int rds_conn_up(struct rds_connection *conn) { diff --git a/net/rds/send.c b/net/rds/send.c index 68e2bdb08fd0..9a529a01cdc6 100644 --- a/net/rds/send.c +++ b/net/rds/send.c @@ -1340,7 +1340,8 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len) goto out; } - rds_conn_path_connect_if_down(cpath); + if (rds_conn_path_down(cpath)) + rds_check_all_paths(conn); ret = rds_cong_wait(conn->c_fcong, dport, nonblock, rs); if (ret) {