b=20288 kiblnd_check_conns can deadlock

Avoid dropping global lock in kiblnd_check_conns

i=maxim
i=liang

diff --git a/lnet/klnds/o2iblnd/o2iblnd.h b/lnet/klnds/o2iblnd/o2iblnd.h
index db98289..bac486d 100644 (file)
--- a/lnet/klnds/o2iblnd/o2iblnd.h
+++ b/lnet/klnds/o2iblnd/o2iblnd.h
@@ -553,41 +553,43 @@ typedef struct kib_connvars
 
 typedef struct kib_conn
 {
 
 typedef struct kib_conn
 {

-        struct kib_peer     *ibc_peer;          /* owning peer */
-        kib_hca_dev_t      *ibc_hdev;           /* HCA bound on */
-        cfs_list_t           ibc_list;          /* stash on peer's conn list */
-        cfs_list_t           ibc_sched_list;    /* schedule for attention */
-        __u16                ibc_version;       /* version of connection */
-        __u64                ibc_incarnation;   /* which instance of the peer */
-        cfs_atomic_t         ibc_refcount;      /* # users */
-        int                  ibc_state;         /* what's happening */
-        int                  ibc_nsends_posted; /* # uncompleted sends */
-        int                  ibc_noops_posted;  /* # uncompleted NOOPs */
-        int                  ibc_credits;       /* # credits I have */
-        int                  ibc_outstanding_credits; /* # credits to return */
-        int                  ibc_reserved_credits;/* # ACK/DONE msg credits */
-        int                  ibc_comms_error;   /* set on comms error */
-        int                  ibc_nrx:16;        /* receive buffers owned */
-        int                  ibc_scheduled:1;   /* scheduled for attention */
-        int                  ibc_ready:1;       /* CQ callback fired */
-        unsigned long        ibc_last_send;     /* time of last send */
-        cfs_list_t           ibc_early_rxs;     /* rxs completed before ESTABLISHED */
-        cfs_list_t          ibc_tx_noops;       /* IBLND_MSG_NOOPs for IBLND_MSG_VERSION_1 */
-        cfs_list_t           ibc_tx_queue;       /* sends that need a credit */
-        cfs_list_t           ibc_tx_queue_nocred;/* sends that don't need a credit */
-        cfs_list_t           ibc_tx_queue_rsrvd; /* sends that need to reserve an ACK/DONE msg */
-        cfs_list_t           ibc_active_txs;     /* active tx awaiting completion */
-        cfs_spinlock_t       ibc_lock;           /* serialise */
-        kib_rx_t            *ibc_rxs;            /* the rx descs */
-        kib_pages_t         *ibc_rx_pages;       /* premapped rx msg pages */
-
-        struct rdma_cm_id   *ibc_cmid;           /* CM id */
-        struct ib_cq        *ibc_cq;             /* completion queue */
-
-        kib_connvars_t      *ibc_connvars;       /* in-progress connection state */
+        struct kib_peer   *ibc_peer;          /* owning peer */
+        kib_hca_dev_t     *ibc_hdev;           /* HCA bound on */
+        cfs_list_t         ibc_list;          /* stash on peer's conn list */
+        cfs_list_t         ibc_sched_list;    /* schedule for attention */
+        cfs_list_t         ibc_connd_list;    /* kiblnd_check_conns only */
+        __u16              ibc_version;       /* version of connection */
+        __u64              ibc_incarnation;   /* which instance of the peer */
+        cfs_atomic_t       ibc_refcount;      /* # users */
+        int                ibc_state;         /* what's happening */
+        int                ibc_nsends_posted; /* # uncompleted sends */
+        int                ibc_noops_posted;  /* # uncompleted NOOPs */
+        int                ibc_credits;       /* # credits I have */
+        int                ibc_outstanding_credits; /* # credits to return */
+        int                ibc_reserved_credits;/* # ACK/DONE msg credits */
+        int                ibc_retry_noop;    /* need to retry returning credits */
+        int                ibc_comms_error;   /* set on comms error */
+        int                ibc_nrx:16;        /* receive buffers owned */
+        int                ibc_scheduled:1;   /* scheduled for attention */
+        int                ibc_ready:1;       /* CQ callback fired */
+        unsigned long      ibc_last_send;     /* time of last send */
+        cfs_list_t         ibc_early_rxs;     /* rxs completed before ESTABLISHED */
+        cfs_list_t         ibc_tx_noops;       /* IBLND_MSG_NOOPs for IBLND_MSG_VERSION_1 */
+        cfs_list_t         ibc_tx_queue;       /* sends that need a credit */
+        cfs_list_t         ibc_tx_queue_nocred;/* sends that don't need a credit */
+        cfs_list_t         ibc_tx_queue_rsrvd; /* sends that need to reserve an ACK/DONE msg */
+        cfs_list_t         ibc_active_txs;     /* active tx awaiting completion */
+        cfs_spinlock_t     ibc_lock;           /* serialise */
+        kib_rx_t          *ibc_rxs;            /* the rx descs */
+        kib_pages_t       *ibc_rx_pages;       /* premapped rx msg pages */
+
+        struct rdma_cm_id *ibc_cmid;           /* CM id */
+        struct ib_cq      *ibc_cq;             /* completion queue */
+
+        kib_connvars_t    *ibc_connvars;       /* in-progress connection state */

 } kib_conn_t;
 
 } kib_conn_t;
 

-#define IBLND_CONN_INIT               0         /* being intialised */
+#define IBLND_CONN_INIT               0         /* being initialised */

 #define IBLND_CONN_ACTIVE_CONNECT     1         /* active sending req */
 #define IBLND_CONN_PASSIVE_WAIT       2         /* passive waiting for rtu */
 #define IBLND_CONN_ESTABLISHED        3         /* connection established */
 #define IBLND_CONN_ACTIVE_CONNECT     1         /* active sending req */
 #define IBLND_CONN_PASSIVE_WAIT       2         /* passive waiting for rtu */
 #define IBLND_CONN_ESTABLISHED        3         /* connection established */

diff --git a/lnet/klnds/o2iblnd/o2iblnd_cb.c b/lnet/klnds/o2iblnd/o2iblnd_cb.c
index 431aa90..35f5b88 100644 (file)
--- a/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -928,6 +928,7 @@ kiblnd_check_sends (kib_conn_t *conn)
                 conn->ibc_reserved_credits--;
         }
 
                 conn->ibc_reserved_credits--;
         }
 

+        conn->ibc_retry_noop = 0;

         if (kiblnd_send_noop(conn)) {
                 cfs_spin_unlock(&conn->ibc_lock);
 
         if (kiblnd_send_noop(conn)) {
                 cfs_spin_unlock(&conn->ibc_lock);
 


@@ -938,6 +939,8 @@ kiblnd_check_sends (kib_conn_t *conn)
                 cfs_spin_lock(&conn->ibc_lock);
                 if (tx != NULL)
                         kiblnd_queue_tx_locked(tx, conn);
                 cfs_spin_lock(&conn->ibc_lock);
                 if (tx != NULL)
                         kiblnd_queue_tx_locked(tx, conn);

+                else if (kiblnd_send_noop(conn)) /* still... */
+                        conn->ibc_retry_noop = 1;

         }
 
         kiblnd_conn_addref(conn); /* 1 ref for me.... (see b21911) */
         }
 
         kiblnd_conn_addref(conn); /* 1 ref for me.... (see b21911) */


@@ -2908,14 +2911,11 @@ kiblnd_cm_callback(struct rdma_cm_id *cmid, struct rdma_cm_event *event)
         }
 }
 
         }
 }
 

-int
-kiblnd_check_txs (kib_conn_t *conn, cfs_list_t *txs)
+static int
+kiblnd_check_txs_locked (kib_conn_t *conn, cfs_list_t *txs)

 {
         kib_tx_t          *tx;
         cfs_list_t        *ttmp;
 {
         kib_tx_t          *tx;
         cfs_list_t        *ttmp;

-        int                timed_out = 0;
-
-        cfs_spin_lock(&conn->ibc_lock);

 
         cfs_list_for_each (ttmp, txs) {
                 tx = cfs_list_entry (ttmp, kib_tx_t, tx_list);
 
         cfs_list_for_each (ttmp, txs) {
                 tx = cfs_list_entry (ttmp, kib_tx_t, tx_list);


@@ -2928,41 +2928,40 @@ kiblnd_check_txs (kib_conn_t *conn, cfs_list_t *txs)
                 }
 
                 if (cfs_time_aftereq (jiffies, tx->tx_deadline)) {
                 }
 
                 if (cfs_time_aftereq (jiffies, tx->tx_deadline)) {

-                        timed_out = 1;

                         CERROR("Timed out tx: %s, %lu seconds\n",
                                kiblnd_queue2str(conn, txs),
                                cfs_duration_sec(jiffies - tx->tx_deadline));
                         CERROR("Timed out tx: %s, %lu seconds\n",
                                kiblnd_queue2str(conn, txs),
                                cfs_duration_sec(jiffies - tx->tx_deadline));

-                        break;
+                        return 1;

                 }
         }
 
                 }
         }
 

-        cfs_spin_unlock(&conn->ibc_lock);
-        return timed_out;
+        return 0;

 }
 
 }
 

-int
-kiblnd_conn_timed_out (kib_conn_t *conn)
+static int
+kiblnd_conn_timed_out_locked (kib_conn_t *conn)

 {
 {

-        return  kiblnd_check_txs(conn, &conn->ibc_tx_queue) ||
-                kiblnd_check_txs(conn, &conn->ibc_tx_noops) ||
-                kiblnd_check_txs(conn, &conn->ibc_tx_queue_rsrvd) ||
-                kiblnd_check_txs(conn, &conn->ibc_tx_queue_nocred) ||
-                kiblnd_check_txs(conn, &conn->ibc_active_txs);
+        return  kiblnd_check_txs_locked(conn, &conn->ibc_tx_queue) ||
+                kiblnd_check_txs_locked(conn, &conn->ibc_tx_noops) ||
+                kiblnd_check_txs_locked(conn, &conn->ibc_tx_queue_rsrvd) ||
+                kiblnd_check_txs_locked(conn, &conn->ibc_tx_queue_nocred) ||
+                kiblnd_check_txs_locked(conn, &conn->ibc_active_txs);

 }
 
 void
 kiblnd_check_conns (int idx)
 {
 }
 
 void
 kiblnd_check_conns (int idx)
 {

-        cfs_list_t        *peers = &kiblnd_data.kib_peers[idx];
-        cfs_list_t        *ptmp;
-        kib_peer_t        *peer;
-        kib_conn_t        *conn;
-        cfs_list_t        *ctmp;
-        unsigned long      flags;
+        CFS_LIST_HEAD (closes);
+        CFS_LIST_HEAD (checksends);
+        cfs_list_t    *peers = &kiblnd_data.kib_peers[idx];
+        cfs_list_t    *ptmp;
+        kib_peer_t    *peer;
+        kib_conn_t    *conn;
+        cfs_list_t    *ctmp;
+        unsigned long  flags;

 
 

- again:

         /* NB. We expect to have a look at all the peers and not find any
         /* NB. We expect to have a look at all the peers and not find any

-         * rdmas to time out, so we just use a shared lock while we
+         * RDMAs to time out, so we just use a shared lock while we

          * take a look... */
         cfs_read_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
 
          * take a look... */
         cfs_read_lock_irqsave(&kiblnd_data.kib_global_lock, flags);
 


@@ -2970,41 +2969,63 @@ kiblnd_check_conns (int idx)
                 peer = cfs_list_entry (ptmp, kib_peer_t, ibp_list);
 
                 cfs_list_for_each (ctmp, &peer->ibp_conns) {
                 peer = cfs_list_entry (ptmp, kib_peer_t, ibp_list);
 
                 cfs_list_for_each (ctmp, &peer->ibp_conns) {

-                        conn = cfs_list_entry (ctmp, kib_conn_t, ibc_list);
+                        int timedout;
+                        int retry_noop;
+
+                        conn = cfs_list_entry(ctmp, kib_conn_t, ibc_list);

 
                         LASSERT (conn->ibc_state == IBLND_CONN_ESTABLISHED);
 
 
                         LASSERT (conn->ibc_state == IBLND_CONN_ESTABLISHED);
 

-                        /* In case we have enough credits to return via a
-                         * NOOP, but there were no non-blocking tx descs
-                         * free to do it last time... */
-                        kiblnd_check_sends(conn);
+                        cfs_spin_lock(&conn->ibc_lock);

 
 

-                        if (!kiblnd_conn_timed_out(conn))
+                        retry_noop = conn->ibc_retry_noop;
+                        timedout   = kiblnd_conn_timed_out_locked(conn);
+                        if (!retry_noop && !timedout) {
+                                cfs_spin_unlock(&conn->ibc_lock);

                                 continue;
                                 continue;

+                        }

 
 

-                        /* Handle timeout by closing the whole connection.  We
-                         * can only be sure RDMA activity has ceased once the
-                         * QP has been modified. */
-
-                        kiblnd_conn_addref(conn); /* 1 ref for me... */
-
-                        cfs_read_unlock_irqrestore(&kiblnd_data.kib_global_lock,
-                                                   flags);
-
-                        CERROR("Timed out RDMA with %s (%lu)\n",
-                               libcfs_nid2str(peer->ibp_nid),
-                               cfs_duration_sec(cfs_time_current() -
-                                                peer->ibp_last_alive));
-
-                        kiblnd_close_conn(conn, -ETIMEDOUT);
-                        kiblnd_conn_decref(conn); /* ...until here */
+                        if (timedout) {
+                                CERROR("Timed out RDMA with %s (%lu): "
+                                       "c: %u, oc: %u, rc: %u\n",
+                                       libcfs_nid2str(peer->ibp_nid),
+                                       cfs_duration_sec(cfs_time_current() -
+                                                        peer->ibp_last_alive),
+                                       conn->ibc_credits,
+                                       conn->ibc_outstanding_credits,
+                                       conn->ibc_reserved_credits);
+                                cfs_list_add(&conn->ibc_connd_list, &closes);
+                        } else {
+                                cfs_list_add(&conn->ibc_connd_list, &checksends);
+                        }
+                        /* +ref for 'closes' or 'checksends' */
+                        kiblnd_conn_addref(conn);

 
 

-                        /* start again now I've dropped the lock */
-                        goto again;
+                        cfs_spin_unlock(&conn->ibc_lock);

                 }
         }
 
         cfs_read_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);
                 }
         }
 
         cfs_read_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags);

+
+        /* Handle timeout by closing the whole
+         * connection. We can only be sure RDMA activity
+         * has ceased once the QP has been modified. */
+        while (!cfs_list_empty(&closes)) {
+                conn = cfs_list_entry(closes.next, kib_conn_t, ibc_connd_list);
+                cfs_list_del(&conn->ibc_connd_list);
+                kiblnd_close_conn(conn, -ETIMEDOUT);
+                kiblnd_conn_decref(conn);
+        }
+
+        /* In case we have enough credits to return via a
+         * NOOP, but there were no non-blocking tx descs
+         * free to do it last time... */
+        while (!cfs_list_empty(&checksends)) {
+                conn = cfs_list_entry(checksends.next, kib_conn_t, ibc_connd_list);
+                cfs_list_del(&conn->ibc_connd_list);
+                kiblnd_check_sends(conn);
+                kiblnd_conn_decref(conn);
+        }

 }
 
 void
 }
 
 void