[PATCH net-next 00/10] net/smc: updates 2017-09-20

Hi Dave,

here is a collection of small smc-patches built for net-next improving
the smc code in different areas.

Thanks,
Ursula

Hans Wippel (2):
  net/smc: add missing dev_put
  net/smc: add receive timeout check

Ursula Braun (8):
  net/smc: take RCU read lock for routing cache lookup
  net/smc: adjust net_device refcount
  net/smc: adapt send request completion notification
  net/smc: longer delay for client link group removal
  net/smc: terminate link group if out-of-sync is received
  net/smc: introduce a delay
  net/smc: parameter cleanup in smc_cdc_get_free_slot()
  net/smc: no close wait in case of process shut down

 net/smc/af_smc.c    | 16 +++++++++-------
 net/smc/smc.h       |  2 +-
 net/smc/smc_cdc.c   |  7 ++++---
 net/smc/smc_cdc.h   |  3 ++-
 net/smc/smc_clc.c   | 10 +++++-----
 net/smc/smc_clc.h   |  3 +--
 net/smc/smc_close.c | 27 +++++++++++++++------------
 net/smc/smc_core.c  | 16 ++++++++++++----
 net/smc/smc_ib.c    |  1 +
 net/smc/smc_pnet.c  |  4 +++-
 net/smc/smc_rx.c    |  2 ++
 net/smc/smc_tx.c    | 18 ++++++++++--------
 net/smc/smc_wr.c    |  2 +-
 13 files changed, 66 insertions(+), 45 deletions(-)

-- 
2.13.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Hans Wippel <redacted>

In the infiniband part, SMC currently uses get_netdev which calls
dev_hold on the returned net device. However, the SMC code never calls
dev_put on that net device resulting in a wrong reference count.

This patch adds a dev_put after the usage of the net device to fix the
issue.

Signed-off-by: Hans Wippel <redacted>
Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_ib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 547e0e113b17..0b5852299158 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c

@@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct smc_ib_device *smcibdev, u8 ibport)
 	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
 	if (ndev) {
 		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
+		dev_put(ndev);
 	} else if (!rc) {
 		memcpy(&smcibdev->mac[ibport - 1][0],
 		       &smcibdev->gid[ibport - 1].raw[8], 3);

-- 
2.13.5

From: Hans Wippel <redacted>

The SMC receive function currently lacks a timeout check under the
condition that no data were received and no data are available. This
patch adds such a check.

Signed-off-by: Hans Wippel <redacted>
Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index b17a333e9bb0..3e631ae4b6b6 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c

@@ -148,6 +148,8 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg, size_t len,
 				read_done = sock_intr_errno(timeo);
 				break;
 			}
+			if (!timeo)
+				return -EAGAIN;
 		}
 
 		if (!atomic_read(&conn->bytes_to_rcv)) {

-- 
2.13.5

smc_pnet_fill_entry() uses dev_get_by_name() adding a refcount to ndev.
The following smc_pnet_enter() has to reduce the refcount if the entry
to be added exists already in the pnet table.

Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_pnet.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 78f7af28ae4f..31f8453c25c5 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c

@@ -181,8 +181,10 @@ static int smc_pnet_enter(struct smc_pnetentry *new_pnetelem)
 			     sizeof(new_pnetelem->ndev->name)) ||
 		    smc_pnet_same_ibname(pnetelem,
 					 new_pnetelem->smcibdev->ibdev->name,
-					 new_pnetelem->ib_port))
+					 new_pnetelem->ib_port)) {
+			dev_put(pnetelem->ndev);
 			goto found;
+		}
 	}
 	list_add_tail(&new_pnetelem->list, &smc_pnettable.pnetlist);
 	rc = 0;

-- 
2.13.5

Client link group creation always follows the server linkgroup creation.
If peer creates a new server link group, client has to create a new
client link group. If peer reuses a server link group for a new
connection, client has to reuse its client link group as well. This
patch introduces a longer delay for client link group removal to make
sure this link group still exists, once the peer decides to reuse a
server link group. This avoids out-of-sync conditions for link groups.
If already scheduled, modify the delay.

Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_core.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 1a16d51e2330..20b66e79c5d6 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c

@@ -25,8 +25,9 @@
 #include "smc_cdc.h"
 #include "smc_close.h"
 
-#define SMC_LGR_NUM_INCR	256
-#define SMC_LGR_FREE_DELAY	(600 * HZ)
+#define SMC_LGR_NUM_INCR		256
+#define SMC_LGR_FREE_DELAY_SERV		(600 * HZ)
+#define SMC_LGR_FREE_DELAY_CLNT		(SMC_LGR_FREE_DELAY_SERV + 10)
 
 static u32 smc_lgr_num;			/* unique link group number */
 

@@ -107,8 +108,15 @@ static void smc_lgr_unregister_conn(struct smc_connection *conn)
 		__smc_lgr_unregister_conn(conn);
 	}
 	write_unlock_bh(&lgr->conns_lock);
-	if (reduced && !lgr->conns_num)
-		schedule_delayed_work(&lgr->free_work, SMC_LGR_FREE_DELAY);
+	if (!reduced || lgr->conns_num)
+		return;
+	/* client link group creation always follows the server link group
+	 * creation. For client use a somewhat higher removal delay time,
+	 * otherwise there is a risk of out-of-sync link groups.
+	 */
+	mod_delayed_work(system_wq, &lgr->free_work,
+			 lgr->role == SMC_CLNT ? SMC_LGR_FREE_DELAY_CLNT :
+						 SMC_LGR_FREE_DELAY_SERV);
 }
 
 static void smc_lgr_free_work(struct work_struct *work)

-- 
2.13.5

An out-of-sync condition can just be detected by the client.
If the server receives a CLC DECLINE message indicating an out-of-sync
condition for the link groups, the server must clean up the out-of-sync
link group.
There is no need for an extra third parameter in smc_clc_send_decline().

Signed-off-by: Ursula Braun <redacted>
---
 net/smc/af_smc.c  |  6 ++----
 net/smc/smc_clc.c | 10 +++++-----
 net/smc/smc_clc.h |  3 +--
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 2e8d2dabac0c..745f145d4c4d 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c

@@ -513,7 +513,7 @@ static int smc_connect_rdma(struct smc_sock *smc)
 	/* RDMA setup failed, switch back to TCP */
 	smc->use_fallback = true;
 	if (reason_code && (reason_code != SMC_CLC_DECL_REPLY)) {
-		rc = smc_clc_send_decline(smc, reason_code, 0);
+		rc = smc_clc_send_decline(smc, reason_code);
 		if (rc < sizeof(struct smc_clc_msg_decline))
 			goto out_err;
 	}

@@ -808,8 +808,6 @@ static void smc_listen_work(struct work_struct *work)
 		rc = local_contact;
 		if (rc == -ENOMEM)
 			reason_code = SMC_CLC_DECL_MEM;/* insufficient memory*/
-		else if (rc == -ENOLINK)
-			reason_code = SMC_CLC_DECL_SYNCERR; /* synchr. error */
 		goto decline_rdma;
 	}
 	link = &new_smc->conn.lgr->lnk[SMC_SINGLE_LINK];

@@ -903,7 +901,7 @@ static void smc_listen_work(struct work_struct *work)
 	smc_conn_free(&new_smc->conn);
 	new_smc->use_fallback = true;
 	if (reason_code && (reason_code != SMC_CLC_DECL_REPLY)) {
-		rc = smc_clc_send_decline(new_smc, reason_code, 0);
+		rc = smc_clc_send_decline(new_smc, reason_code);
 		if (rc < sizeof(struct smc_clc_msg_decline))
 			goto out_err;
 	}

diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 3934913ab835..b7dd2743fb5c 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c

@@ -95,9 +95,10 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 	}
 	if (clcm->type == SMC_CLC_DECLINE) {
 		reason_code = SMC_CLC_DECL_REPLY;
-		if (ntohl(((struct smc_clc_msg_decline *)buf)->peer_diagnosis)
-			== SMC_CLC_DECL_SYNCERR)
+		if (((struct smc_clc_msg_decline *)buf)->hdr.flag) {
 			smc->conn.lgr->sync_err = true;
+			smc_lgr_terminate(smc->conn.lgr);
+		}
 	}
 
 out:

@@ -105,8 +106,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 }
 
 /* send CLC DECLINE message across internal TCP socket */
-int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info,
-			 u8 out_of_sync)
+int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info)
 {
 	struct smc_clc_msg_decline dclc;
 	struct msghdr msg;

@@ -118,7 +118,7 @@ int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info,
 	dclc.hdr.type = SMC_CLC_DECLINE;
 	dclc.hdr.length = htons(sizeof(struct smc_clc_msg_decline));
 	dclc.hdr.version = SMC_CLC_V1;
-	dclc.hdr.flag = out_of_sync ? 1 : 0;
+	dclc.hdr.flag = (peer_diag_info == SMC_CLC_DECL_SYNCERR) ? 1 : 0;
 	memcpy(dclc.id_for_peer, local_systemid, sizeof(local_systemid));
 	dclc.peer_diagnosis = htonl(peer_diag_info);
 	memcpy(dclc.trl.eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));

diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index 13db8ce177c9..1c55414041d4 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h

@@ -106,8 +106,7 @@ struct smc_ib_device;
 
 int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 		     u8 expected_type);
-int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info,
-			 u8 out_of_sync);
+int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info);
 int smc_clc_send_proposal(struct smc_sock *smc, struct smc_ib_device *smcibdev,
 			  u8 ibport);
 int smc_clc_send_confirm(struct smc_sock *smc);

-- 
2.13.5

The solicited flag is meaningful for the receive completion queue.
Ask for next work completion of any type on the send queue.

Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_wr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index ab56bda66783..525d91e0d57e 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c

@@ -244,7 +244,7 @@ int smc_wr_tx_send(struct smc_link *link, struct smc_wr_tx_pend_priv *priv)
 	int rc;
 
 	ib_req_notify_cq(link->smcibdev->roce_cq_send,
-			 IB_CQ_SOLICITED_MASK | IB_CQ_REPORT_MISSED_EVENTS);
+			 IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS);
 	pend = container_of(priv, struct smc_wr_tx_pend, priv);
 	rc = ib_post_send(link->roce_qp, &link->wr_tx_ibs[pend->idx],
 			  &failed_wr);

-- 
2.13.5

The number of outstanding work requests is limited. If all work
requests are in use, tx processing is postponed to another scheduling
of the tx worker. Switch to a delayed worker to have a gap for tx
completion queue events before the next retry.

Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc.h       |  2 +-
 net/smc/smc_close.c | 12 +++++++-----
 net/smc/smc_tx.c    | 12 ++++++++----
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/smc/smc.h b/net/smc/smc.h
index 6e44313e4467..0ccd6fa387ad 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h

@@ -149,7 +149,7 @@ struct smc_connection {
 	atomic_t		sndbuf_space;	/* remaining space in sndbuf */
 	u16			tx_cdc_seq;	/* sequence # for CDC send */
 	spinlock_t		send_lock;	/* protect wr_sends */
-	struct work_struct	tx_work;	/* retry of smc_cdc_msg_send */
+	struct delayed_work	tx_work;	/* retry of smc_cdc_msg_send */
 
 	struct smc_host_cdc_msg	local_rx_ctrl;	/* filled during event_handl.
 						 * .prod cf. TCP rcv_nxt

diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 3c2e166b5d22..5201bc103bd8 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c

@@ -208,7 +208,7 @@ int smc_close_active(struct smc_sock *smc)
 	case SMC_ACTIVE:
 		smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		if (sk->sk_state == SMC_ACTIVE) {
 			/* send close request */

@@ -234,7 +234,7 @@ int smc_close_active(struct smc_sock *smc)
 		if (!smc_cdc_rxed_any_close(conn))
 			smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		if (sk->sk_err != ECONNABORTED) {
 			/* confirm close from peer */

@@ -263,7 +263,9 @@ int smc_close_active(struct smc_sock *smc)
 		/* peer sending PeerConnectionClosed will cause transition */
 		break;
 	case SMC_PROCESSABORT:
-		cancel_work_sync(&conn->tx_work);
+		release_sock(sk);
+		cancel_delayed_work_sync(&conn->tx_work);
+		lock_sock(sk);
 		smc_close_abort(conn);
 		sk->sk_state = SMC_CLOSED;
 		smc_close_wait_tx_pends(smc);

@@ -425,7 +427,7 @@ int smc_close_shutdown_write(struct smc_sock *smc)
 	case SMC_ACTIVE:
 		smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		/* send close wr request */
 		rc = smc_close_wr(conn);

@@ -439,7 +441,7 @@ int smc_close_shutdown_write(struct smc_sock *smc)
 		if (!smc_cdc_rxed_any_close(conn))
 			smc_close_stream_wait(smc, timeout);
 		release_sock(sk);
-		cancel_work_sync(&conn->tx_work);
+		cancel_delayed_work_sync(&conn->tx_work);
 		lock_sock(sk);
 		/* confirm close from peer */
 		rc = smc_close_wr(conn);

diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3c656beb8820..3866573288dd 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c

@@ -24,6 +24,8 @@
 #include "smc_cdc.h"
 #include "smc_tx.h"
 
+#define SMC_TX_WORK_DELAY	HZ
+
 /***************************** sndbuf producer *******************************/
 
 /* callback implementation for sk.sk_write_space()

@@ -406,7 +408,8 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)
 				goto out_unlock;
 			}
 			rc = 0;
-			schedule_work(&conn->tx_work);
+			schedule_delayed_work(&conn->tx_work,
+					      SMC_TX_WORK_DELAY);
 		}
 		goto out_unlock;
 	}

@@ -430,7 +433,7 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)
  */
 static void smc_tx_work(struct work_struct *work)
 {
-	struct smc_connection *conn = container_of(work,
+	struct smc_connection *conn = container_of(to_delayed_work(work),
 						   struct smc_connection,
 						   tx_work);
 	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);

@@ -468,7 +471,8 @@ void smc_tx_consumer_update(struct smc_connection *conn)
 		if (!rc)
 			rc = smc_cdc_msg_send(conn, wr_buf, pend);
 		if (rc < 0) {
-			schedule_work(&conn->tx_work);
+			schedule_delayed_work(&conn->tx_work,
+					      SMC_TX_WORK_DELAY);
 			return;
 		}
 		smc_curs_write(&conn->rx_curs_confirmed,

@@ -487,6 +491,6 @@ void smc_tx_consumer_update(struct smc_connection *conn)
 void smc_tx_init(struct smc_sock *smc)
 {
 	smc->sk.sk_write_space = smc_tx_write_space;
-	INIT_WORK(&smc->conn.tx_work, smc_tx_work);
+	INIT_DELAYED_WORK(&smc->conn.tx_work, smc_tx_work);
 	spin_lock_init(&smc->conn.send_lock);
 }

-- 
2.13.5

Use the smc_connection as first parameter with smc_cdc_get_free_slot().
This is just a small code cleanup, no functional change.

Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_cdc.c | 7 ++++---
 net/smc/smc_cdc.h | 3 ++-
 net/smc/smc_tx.c  | 6 ++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index a7294edbc221..5ef97e5a5f78 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c

@@ -62,10 +62,12 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd,
 	bh_unlock_sock(&smc->sk);
 }
 
-int smc_cdc_get_free_slot(struct smc_link *link,
+int smc_cdc_get_free_slot(struct smc_connection *conn,
 			  struct smc_wr_buf **wr_buf,
 			  struct smc_cdc_tx_pend **pend)
 {
+	struct smc_link *link = &conn->lgr->lnk[SMC_SINGLE_LINK];
+
 	return smc_wr_tx_get_free_slot(link, smc_cdc_tx_handler, wr_buf,
 				       (struct smc_wr_tx_pend_priv **)pend);
 }

@@ -118,8 +120,7 @@ int smc_cdc_get_slot_and_msg_send(struct smc_connection *conn)
 	struct smc_wr_buf *wr_buf;
 	int rc;
 
-	rc = smc_cdc_get_free_slot(&conn->lgr->lnk[SMC_SINGLE_LINK], &wr_buf,
-				   &pend);
+	rc = smc_cdc_get_free_slot(conn, &wr_buf, &pend);
 	if (rc)
 		return rc;
 

diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 8e1d76f26007..56f883d1159c 100644
--- a/net/smc/smc_cdc.h
+++ b/net/smc/smc_cdc.h

@@ -206,7 +206,8 @@ static inline void smc_cdc_msg_to_host(struct smc_host_cdc_msg *local,
 
 struct smc_cdc_tx_pend;
 
-int smc_cdc_get_free_slot(struct smc_link *link, struct smc_wr_buf **wr_buf,
+int smc_cdc_get_free_slot(struct smc_connection *conn,
+			  struct smc_wr_buf **wr_buf,
 			  struct smc_cdc_tx_pend **pend);
 void smc_cdc_tx_dismiss_slots(struct smc_connection *conn);
 int smc_cdc_msg_send(struct smc_connection *conn, struct smc_wr_buf *wr_buf,

diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3866573288dd..ec49bc3c3283 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c

@@ -396,8 +396,7 @@ int smc_tx_sndbuf_nonempty(struct smc_connection *conn)
 	int rc;
 
 	spin_lock_bh(&conn->send_lock);
-	rc = smc_cdc_get_free_slot(&conn->lgr->lnk[SMC_SINGLE_LINK], &wr_buf,
-				   &pend);
+	rc = smc_cdc_get_free_slot(conn, &wr_buf, &pend);
 	if (rc < 0) {
 		if (rc == -EBUSY) {
 			struct smc_sock *smc =

@@ -466,8 +465,7 @@ void smc_tx_consumer_update(struct smc_connection *conn)
 	    ((to_confirm > conn->rmbe_update_limit) &&
 	     ((to_confirm > (conn->rmbe_size / 2)) ||
 	      conn->local_rx_ctrl.prod_flags.write_blocked))) {
-		rc = smc_cdc_get_free_slot(&conn->lgr->lnk[SMC_SINGLE_LINK],
-					   &wr_buf, &pend);
+		rc = smc_cdc_get_free_slot(conn, &wr_buf, &pend);
 		if (!rc)
 			rc = smc_cdc_msg_send(conn, wr_buf, pend);
 		if (rc < 0) {

-- 
2.13.5

Usually socket closing is delayed if there is still data available in
the send buffer to be transmitted. If a process is killed, the delay
should be avoided.

Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_close.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 5201bc103bd8..f0d16fb825f7 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c

@@ -174,15 +174,15 @@ int smc_close_active(struct smc_sock *smc)
 {
 	struct smc_cdc_conn_state_flags *txflags =
 		&smc->conn.local_tx_ctrl.conn_state_flags;
-	long timeout = SMC_MAX_STREAM_WAIT_TIMEOUT;
 	struct smc_connection *conn = &smc->conn;
 	struct sock *sk = &smc->sk;
 	int old_state;
+	long timeout;
 	int rc = 0;
 
-	if (sock_flag(sk, SOCK_LINGER) &&
-	    !(current->flags & PF_EXITING))
-		timeout = sk->sk_lingertime;
+	timeout = current->flags & PF_EXITING ?
+		  0 : sock_flag(sk, SOCK_LINGER) ?
+		      sk->sk_lingertime : SMC_MAX_STREAM_WAIT_TIMEOUT;
 
 again:
 	old_state = sk->sk_state;

@@ -413,13 +413,14 @@ void smc_close_sock_put_work(struct work_struct *work)
 int smc_close_shutdown_write(struct smc_sock *smc)
 {
 	struct smc_connection *conn = &smc->conn;
-	long timeout = SMC_MAX_STREAM_WAIT_TIMEOUT;
 	struct sock *sk = &smc->sk;
 	int old_state;
+	long timeout;
 	int rc = 0;
 
-	if (sock_flag(sk, SOCK_LINGER))
-		timeout = sk->sk_lingertime;
+	timeout = current->flags & PF_EXITING ?
+		  0 : sock_flag(sk, SOCK_LINGER) ?
+		      sk->sk_lingertime : SMC_MAX_STREAM_WAIT_TIMEOUT;
 
 again:
 	old_state = sk->sk_state;

-- 
2.13.5

On Wed, Sep 20, 2017 at 01:58:11PM +0200, Ursula Braun wrote:

The number of outstanding work requests is limited. If all work
requests are in use, tx processing is postponed to another scheduling
of the tx worker. Switch to a delayed worker to have a gap for tx
completion queue events before the next retry.

How will delay prevent and protect the resource exhausting?

Thanks

On 09/20/2017 04:03 PM, Leon Romanovsky wrote:

On Wed, Sep 20, 2017 at 01:58:11PM +0200, Ursula Braun wrote:

quoted

The number of outstanding work requests is limited. If all work
requests are in use, tx processing is postponed to another scheduling
of the tx worker. Switch to a delayed worker to have a gap for tx
completion queue events before the next retry.

How will delay prevent and protect the resource exhausting?

Thanks

SMC runs with a fixed number of in-flight work requests per QP (constant
SMC_WR_BUF_CNT) to prevent resource exhausting. If all work requests are
currently in use, sending of another work request has to wait till some
outstanding work request is confirmed via send completion queue. If sending
is done in a context which is not allowed to wait, the tx_worker is
scheduled instead.
With this patch a small delay is added to avoid too many unsuccessful send
retries due to a still ongoing "all work requests in use" condition.

From: Ursula Braun <redacted>
Date: Wed, 20 Sep 2017 13:58:03 +0200

here is a collection of small smc-patches built for net-next
improving the smc code in different areas.

There are bug fixes in here, which should be targetted at 'net'.

Hi Ursula, Dave, Hans,

I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not coded correctly.
Few fixes are needed.
1. ULP such as SMC should not open code/deference any function pointer like get_netdev() of the IB device.
2. Replace ib_query_gid(..., NULL)
With
ib_query_gid(..., gid_attr);

Use gid_attr.ndev to get the MAC address.
Do dev_put(gid_attr.ndev);

Code should look like below,

struct ib_gid_attr gid_attr;

rc = ib_query_gid(..., &gid_attr);
if (rc || !gid_addr.ndev)
	return -ENODEV;
else
	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
dev_put(gid_addr.ndev);

-----Original Message-----
From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
owner@vger.kernel.org] On Behalf Of Parav Pandit
Sent: Monday, October 02, 2017 3:36 PM
To: Ursula Braun <redacted>; davem@davemloft.net
Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com
Subject: RE: [PATCH net-next 01/10] net/smc: add missing dev_put

Hi Ursula, Dave, Hans,

quoted

-----Original Message-----
From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
owner@vger.kernel.org] On Behalf Of Ursula Braun
Sent: Wednesday, September 20, 2017 6:58 AM
To: davem@davemloft.net
Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
ubraun@linux.vnet.ibm.com
Subject: [PATCH net-next 01/10] net/smc: add missing dev_put

From: Hans Wippel <redacted>

In the infiniband part, SMC currently uses get_netdev which calls
dev_hold on the returned net device. However, the SMC code never calls
dev_put on that net device resulting in a wrong reference count.

This patch adds a dev_put after the usage of the net device to fix the issue.

Signed-off-by: Hans Wippel <redacted>
Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_ib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index

547e0e113b17..0b5852299158 100644

--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c

@@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct

smc_ib_device *smcibdev, u8 ibport)
 	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
 	if (ndev) {
 		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
+		dev_put(ndev);

I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not coded
correctly.
Few fixes are needed.
1. ULP such as SMC should not open code/deference any function pointer like
get_netdev() of the IB device.
2. Replace ib_query_gid(..., NULL)
With
ib_query_gid(..., gid_attr);

Use gid_attr.ndev to get the MAC address.
Do dev_put(gid_attr.ndev);

Code should look like below,

struct ib_gid_attr gid_attr;

rc = ib_query_gid(..., &gid_attr);
if (rc || !gid_addr.ndev)
	return -ENODEV;
else
	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
dev_put(gid_addr.ndev);
--

Also,
smc_link_determine_gid() doesn't do dev_put(gattr.ndev) in for loop.
Please fix it as well.

On 10/02/2017 10:36 PM, Parav Pandit wrote:

Hi Ursula, Dave, Hans,

quoted

-----Original Message-----
From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
owner@vger.kernel.org] On Behalf Of Ursula Braun
Sent: Wednesday, September 20, 2017 6:58 AM
To: davem@davemloft.net
Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
ubraun@linux.vnet.ibm.com
Subject: [PATCH net-next 01/10] net/smc: add missing dev_put

From: Hans Wippel <redacted>

In the infiniband part, SMC currently uses get_netdev which calls dev_hold on
the returned net device. However, the SMC code never calls dev_put on that net
device resulting in a wrong reference count.

This patch adds a dev_put after the usage of the net device to fix the issue.

Signed-off-by: Hans Wippel <redacted>
Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_ib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index

547e0e113b17..0b5852299158 100644

--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c

@@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct smc_ib_device

*smcibdev, u8 ibport)
 	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
 	if (ndev) {
 		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
+		dev_put(ndev);

I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not coded correctly.
Few fixes are needed.
1. ULP such as SMC should not open code/deference any function pointer like get_netdev() of the IB device.
2. Replace ib_query_gid(..., NULL)
With
ib_query_gid(..., gid_attr);

Use gid_attr.ndev to get the MAC address.
Do dev_put(gid_attr.ndev);

Code should look like below,

struct ib_gid_attr gid_attr;

rc = ib_query_gid(..., &gid_attr);
if (rc || !gid_addr.ndev)
	return -ENODEV;
else
	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
dev_put(gid_addr.ndev);

Thanks, Parav!
Following your fix ideas I plan to change the function into this one:

static int smc_ib_fill_gid_and_mac(struct smc_ib_device *smcibdev, u8 ibport)
{
        struct ib_gid_attr gattr;
        int rc;

        rc = ib_query_gid(smcibdev->ibdev, ibport, 0,
                          &smcibdev->gid[ibport - 1], &gattr);
        /* the SMC protocol requires specification of the roce MAC address;
         * if net_device cannot be determined, it can be derived from gid 0
         */
        if (rc)
                return rc;

        if (gattr.ndev) {
                memcpy(&smcibdev->mac, gattr.ndev->dev_addr, ETH_ALEN);
                dev_put(gattr.ndev);
        } else {
                memcpy(&smcibdev->mac[ibport - 1][0],
                       &smcibdev->gid[ibport - 1].raw[8], 3);
                memcpy(&smcibdev->mac[ibport - 1][3],
                       &smcibdev->gid[ibport - 1].raw[13], 3);
                smcibdev->mac[ibport - 1][0] &= ~0x02;
        }
        return 0;
}

If you agree, I will submit the corresponding patch including a
	Suggested-by: Parav Pandit [off-list ref]

Regards, Ursula

Hi Ursula,

-----Original Message-----
From: Ursula Braun [mailto:ubraun@linux.vnet.ibm.com]
Sent: Thursday, October 05, 2017 2:54 AM
To: Parav Pandit <redacted>
Cc: davem@davemloft.net; netdev@vger.kernel.org; linux-
rdma@vger.kernel.org; linux-s390@vger.kernel.org; jwi@linux.vnet.ibm.com;
schwidefsky@de.ibm.com; heiko.carstens@de.ibm.com;
raspl@linux.vnet.ibm.com
Subject: Re: [PATCH net-next 01/10] net/smc: add missing dev_put



On 10/02/2017 10:36 PM, Parav Pandit wrote:

quoted

Hi Ursula, Dave, Hans,

quoted

-----Original Message-----
From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
owner@vger.kernel.org] On Behalf Of Ursula Braun
Sent: Wednesday, September 20, 2017 6:58 AM
To: davem@davemloft.net
Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
ubraun@linux.vnet.ibm.com
Subject: [PATCH net-next 01/10] net/smc: add missing dev_put

From: Hans Wippel <redacted>

In the infiniband part, SMC currently uses get_netdev which calls
dev_hold on the returned net device. However, the SMC code never
calls dev_put on that net device resulting in a wrong reference count.

This patch adds a dev_put after the usage of the net device to fix the issue.

Signed-off-by: Hans Wippel <redacted>
Signed-off-by: Ursula Braun <redacted>
---
 net/smc/smc_ib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index

547e0e113b17..0b5852299158 100644

--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c

@@ -380,6 +380,7 @@ static int smc_ib_fill_gid_and_mac(struct

smc_ib_device *smcibdev, u8 ibport)
 	ndev = smcibdev->ibdev->get_netdev(smcibdev->ibdev, ibport);
 	if (ndev) {
 		memcpy(&smcibdev->mac, ndev->dev_addr, ETH_ALEN);
+		dev_put(ndev);

I am sorry for providing late comments. smc_ib_fill_gid_and_mac() is not

coded correctly.

quoted

Few fixes are needed.
1. ULP such as SMC should not open code/deference any function pointer like

get_netdev() of the IB device.

quoted

2. Replace ib_query_gid(..., NULL)
With
ib_query_gid(..., gid_attr);

Use gid_attr.ndev to get the MAC address.
Do dev_put(gid_attr.ndev);

Code should look like below,

struct ib_gid_attr gid_attr;

rc = ib_query_gid(..., &gid_attr);
if (rc || !gid_addr.ndev)
	return -ENODEV;
else
	memcpy(smcibdev->mac, ndev->dev_addr, ETH_ALEN);
dev_put(gid_addr.ndev);

Thanks, Parav!
Following your fix ideas I plan to change the function into this one:

static int smc_ib_fill_gid_and_mac(struct smc_ib_device *smcibdev, u8 ibport) {
        struct ib_gid_attr gattr;
        int rc;

        rc = ib_query_gid(smcibdev->ibdev, ibport, 0,
                          &smcibdev->gid[ibport - 1], &gattr);
        /* the SMC protocol requires specification of the roce MAC address;
         * if net_device cannot be determined, it can be derived from gid 0
         */
        if (rc)
                return rc;

        if (gattr.ndev) {
                memcpy(&smcibdev->mac, gattr.ndev->dev_addr, ETH_ALEN);
                dev_put(gattr.ndev);
        } else {
                memcpy(&smcibdev->mac[ibport - 1][0],
                       &smcibdev->gid[ibport - 1].raw[8], 3);
                memcpy(&smcibdev->mac[ibport - 1][3],
                       &smcibdev->gid[ibport - 1].raw[13], 3);
                smcibdev->mac[ibport - 1][0] &= ~0x02;

This else part is not needed. You can fail the call as suggested above, inline below too.
if (rc || !gid_addr.ndev)
 	return -ENODEV;
There must be valid netdev for RoCE gid.
Rest code looks fine.