Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/iscsi Lock correctly around CV calls.



details:   https://anonhg.NetBSD.org/src/rev/5d7edd9df6a0
branches:  trunk
changeset: 349831:5d7edd9df6a0
user:      mlelstv <mlelstv%NetBSD.org@localhost>
date:      Sun Dec 25 06:55:28 2016 +0000

description:
Lock correctly around CV calls.
Fix handling of session termination.
Enable MPSAFE processing for scsipi.

diffstat:

 sys/dev/iscsi/iscsi_ioctl.c |  47 +++++++++++++++++++++++++++-----------------
 sys/dev/iscsi/iscsi_main.c  |   3 +-
 sys/dev/iscsi/iscsi_rcv.c   |   4 ++-
 sys/dev/iscsi/iscsi_send.c  |  28 ++++++++++++++++----------
 sys/dev/iscsi/iscsi_utils.c |  27 +++++++++++--------------
 5 files changed, 62 insertions(+), 47 deletions(-)

diffs (truncated from 381 to 300 lines):

diff -r 402399204dc0 -r 5d7edd9df6a0 sys/dev/iscsi/iscsi_ioctl.c
--- a/sys/dev/iscsi/iscsi_ioctl.c       Sun Dec 25 06:37:50 2016 +0000
+++ b/sys/dev/iscsi/iscsi_ioctl.c       Sun Dec 25 06:55:28 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: iscsi_ioctl.c,v 1.22 2016/06/15 04:30:30 mlelstv Exp $ */
+/*     $NetBSD: iscsi_ioctl.c,v 1.23 2016/12/25 06:55:28 mlelstv Exp $ */
 
 /*-
  * Copyright (c) 2004,2005,2006,2011 The NetBSD Foundation, Inc.
@@ -128,10 +128,9 @@
 
        was_empty = TAILQ_FIRST(&event_handlers) == NULL;
        TAILQ_INSERT_TAIL(&event_handlers, handler, link);
-       mutex_exit(&iscsi_cleanup_mtx);
-
        if (was_empty)
                iscsi_notify_cleanup();
+       mutex_exit(&iscsi_cleanup_mtx);
 
        par->status = ISCSI_STATUS_SUCCESS;
        DEB(5, ("Register Event OK, ID %d\n", par->event_id));
@@ -162,8 +161,6 @@
        }
 
        TAILQ_REMOVE(&event_handlers, handler, link);
-       mutex_exit(&iscsi_cleanup_mtx);
-
        if (handler->waiter != NULL) {
                handler->waiter->status = ISCSI_STATUS_EVENT_DEREGISTERED;
                cv_broadcast(&iscsi_event_cv);
@@ -173,6 +170,7 @@
                TAILQ_REMOVE(&handler->events, evt, link);
                free(evt, M_TEMP);
        }
+       mutex_exit(&iscsi_cleanup_mtx);
 
        free(handler, M_DEVBUF);
        par->status = ISCSI_STATUS_SUCCESS;
@@ -490,6 +488,7 @@
 kill_connection(connection_t *conn, uint32_t status, int logout, bool recover)
 {
        session_t *sess = conn->session;
+       int terminating;
 
        DEBC(conn, 1, ("Kill_connection: terminating=%d, status=%d, logout=%d, "
                           "state=%d\n",
@@ -516,19 +515,18 @@
                        DEBC(conn, 1, ("Kill_connection setting destroy flag\n"));
                        conn->destroy = TRUE;
                }
-               /* in case it was already terminated earlier and rcv/send-threads */
-               /* are waiting */
-               cv_broadcast(&conn->idle_cv);
        }
 
+       terminating = conn->terminating;
+       if (!terminating)
+               conn->terminating = status;
+       mutex_exit(&iscsi_cleanup_mtx);
+
        /* Don't recurse */
-       if (conn->terminating) {
-               mutex_exit(&iscsi_cleanup_mtx);
+       if (terminating) {
                DEBC(conn, 1, ("Kill_connection exiting (already terminating)\n"));
-               return;
+               goto done;
        }
-       conn->terminating = status;
-       mutex_exit(&iscsi_cleanup_mtx);
 
        if (conn->state == ST_FULL_FEATURE) {
                sess->active_connections--;
@@ -566,8 +564,11 @@
 
        conn->state = ST_SETTLING;
 
+done:
        /* let send thread take over next step of cleanup */
+       mutex_enter(&conn->lock);
        cv_broadcast(&conn->conn_cv);
+       mutex_exit(&conn->lock);
 
        DEBC(conn, 5, ("kill_connection returns\n"));
 }
@@ -899,10 +900,9 @@
                suspend_ccb(ccb, FALSE);
                TAILQ_INSERT_TAIL(&old_waiting, ccb, chain);
        }
-       mutex_exit(&connection->lock);
-
        init_sernum(&connection->StatSN_buf);
        cv_broadcast(&connection->idle_cv);
+       mutex_exit(&connection->lock);
 
        if ((rc = send_login(connection)) != 0) {
                DEBOUT(("Login failed (rc %d)\n", rc));
@@ -954,7 +954,9 @@
                }
        }
 
+       mutex_enter(&session->lock);
        cv_broadcast(&session->sess_cv);
+       mutex_exit(&session->lock);
 
        DEBC(connection, 0, ("Connection ReCreated successfully - status %d\n",
                                                 par->status));
@@ -1631,8 +1633,8 @@
 {
        mutex_enter(&iscsi_cleanup_mtx);
        TAILQ_INSERT_TAIL(&iscsi_cleanupc_list, conn, connections);
+       iscsi_notify_cleanup();
        mutex_exit(&iscsi_cleanup_mtx);
-       iscsi_notify_cleanup();
 }
 
 /*
@@ -1646,8 +1648,8 @@
        mutex_enter(&iscsi_cleanup_mtx);
        conn->timedout = TOUT_QUEUED;
        TAILQ_INSERT_TAIL(&iscsi_timeout_conn_list, conn, tchain);
+       iscsi_notify_cleanup();
        mutex_exit(&iscsi_cleanup_mtx);
-       iscsi_notify_cleanup();
 }
 
 void            
@@ -1685,8 +1687,8 @@
        mutex_enter(&iscsi_cleanup_mtx);
        ccb->timedout = TOUT_QUEUED;
        TAILQ_INSERT_TAIL(&iscsi_timeout_ccb_list, ccb, tchain);
+       iscsi_notify_cleanup();
        mutex_exit(&iscsi_cleanup_mtx);
-       iscsi_notify_cleanup();
 }
 
 void    
@@ -1773,6 +1775,13 @@
 
                        if (--sess->total_connections == 0) {
                                DEB(1, ("Cleanup: session %d\n", sess->id));
+                               if (!sess->terminating) {
+                                       sess->terminating = ISCSI_CONNECTION_TERMINATED;
+                                       KASSERT(sess->sessions.tqe_prev != NULL);
+                                       TAILQ_REMOVE(&iscsi_sessions, sess, sessions);
+                                       sess->sessions.tqe_next = NULL;
+                                       sess->sessions.tqe_prev = NULL;
+                               }
                                KASSERT(sess->sessions.tqe_prev == NULL);
                                TAILQ_INSERT_HEAD(&iscsi_cleanups_list, sess, sessions);
                        }
@@ -1896,6 +1905,8 @@
 void
 iscsi_notify_cleanup(void)
 {
+       KASSERT(mutex_owned(&iscsi_cleanup_mtx));
+
        cv_signal(&iscsi_cleanup_cv);
 }
 
diff -r 402399204dc0 -r 5d7edd9df6a0 sys/dev/iscsi/iscsi_main.c
--- a/sys/dev/iscsi/iscsi_main.c        Sun Dec 25 06:37:50 2016 +0000
+++ b/sys/dev/iscsi/iscsi_main.c        Sun Dec 25 06:55:28 2016 +0000
@@ -367,6 +367,7 @@
        adapt->adapt_minphys = iscsi_minphys;
        adapt->adapt_openings = session->send_window;
        adapt->adapt_max_periph = CCBS_FOR_SCSIPI;
+       adapt->adapt_flags = SCSIPI_ADAPT_MPSAFE;
 
        /*
         * Fill in the scsipi_channel.
@@ -595,9 +596,7 @@
                }
 
                DEB(99, ("Calling scsipi_done (%p), err = %d\n", xs, xs->error));
-               KERNEL_LOCK(1, curlwp);
                scsipi_done(xs);
-               KERNEL_UNLOCK_ONE(curlwp);
                DEB(99, ("scsipi_done returned\n"));
        } else {
                DEBOUT(("ISCSI: iscsi_done CCB %p without XS\n", ccb));
diff -r 402399204dc0 -r 5d7edd9df6a0 sys/dev/iscsi/iscsi_rcv.c
--- a/sys/dev/iscsi/iscsi_rcv.c Sun Dec 25 06:37:50 2016 +0000
+++ b/sys/dev/iscsi/iscsi_rcv.c Sun Dec 25 06:55:28 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: iscsi_rcv.c,v 1.22 2016/06/15 04:33:52 mlelstv Exp $   */
+/*     $NetBSD: iscsi_rcv.c,v 1.23 2016/12/25 06:55:28 mlelstv Exp $   */
 
 /*-
  * Copyright (c) 2004,2005,2006,2011 The NetBSD Foundation, Inc.
@@ -578,7 +578,9 @@
                connection_timeout_stop(conn);
 
                /* let send thread take over next step of cleanup */
+               mutex_enter(&conn->lock);
                cv_broadcast(&conn->conn_cv);
+               mutex_exit(&conn->lock);
        }
 
        return !otherconn;
diff -r 402399204dc0 -r 5d7edd9df6a0 sys/dev/iscsi/iscsi_send.c
--- a/sys/dev/iscsi/iscsi_send.c        Sun Dec 25 06:37:50 2016 +0000
+++ b/sys/dev/iscsi/iscsi_send.c        Sun Dec 25 06:55:28 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: iscsi_send.c,v 1.31 2016/06/15 04:30:30 mlelstv Exp $  */
+/*     $NetBSD: iscsi_send.c,v 1.32 2016/12/25 06:55:28 mlelstv Exp $  */
 
 /*-
  * Copyright (c) 2004,2005,2006,2011 The NetBSD Foundation, Inc.
@@ -408,7 +408,9 @@
        } while (!conn->destroy);
 
        /* wake up anyone waiting for a PDU */
+       mutex_enter(&conn->lock);
        cv_broadcast(&conn->conn_cv);
+       mutex_exit(&conn->lock);
 
        /* wake up any waiting CCBs */
        while ((ccb = TAILQ_FIRST(&conn->ccbs_waiting)) != NULL) {
@@ -468,6 +470,8 @@
 
        mutex_enter(&conn->lock);
        if (pdisp == PDUDISP_WAIT) {
+               KASSERT(ccb != NULL);
+
                ccb->pdu_waiting = pdu;
 
                /* save UIO and IOVEC for retransmit */
@@ -483,16 +487,20 @@
                TAILQ_INSERT_HEAD(&conn->pdus_to_send, pdu, send_chain);
        else
                TAILQ_INSERT_TAIL(&conn->pdus_to_send, pdu, send_chain);
-       mutex_exit(&conn->lock);
 
        cv_broadcast(&conn->conn_cv);
 
        if (cdisp != CCBDISP_NOWAIT) {
-               ccb_timeout_start(ccb, COMMAND_TIMEOUT);
+               KASSERT(ccb != NULL);
+               KASSERTMSG(ccb->connection == conn, "conn mismatch %p != %p\n", ccb->connection, conn);
 
-               mutex_enter(&conn->lock);
                if (prev_cdisp <= CCBDISP_NOWAIT)
                        suspend_ccb(ccb, TRUE);
+
+               mutex_exit(&conn->lock);
+               ccb_timeout_start(ccb, COMMAND_TIMEOUT);
+               mutex_enter(&conn->lock);
+
                while (ccb->disp == CCBDISP_WAIT) {
                        DEBC(conn, 15, ("Send_pdu: ccb=%p cdisp=%d waiting\n",
                                ccb, ccb->disp));
@@ -500,8 +508,9 @@
                        DEBC(conn, 15, ("Send_pdu: ccb=%p cdisp=%d returned\n",
                                ccb, ccb->disp));
                }
-               mutex_exit(&conn->lock);
        }
+
+       mutex_exit(&conn->lock);
 }
 
 
@@ -546,9 +555,8 @@
                TAILQ_INSERT_TAIL(&conn->pdus_to_send, pdu, send_chain);
        }
        ccb_timeout_start(ccb, COMMAND_TIMEOUT);
+       cv_broadcast(&conn->conn_cv);
        mutex_exit(&conn->lock);
-
-       cv_broadcast(&conn->conn_cv);
 }
 
 
@@ -872,8 +880,8 @@
                        handle_connection_error(conn, rc, LOGOUT_CONNECTION);
                } else if (tx_pdu != NULL) {
                        init_text_pdu(conn, tx_ccb, tx_pdu, rx_pdu);
-                       setup_tx_uio(tx_pdu, tx_pdu->temp_data_len, tx_pdu->temp_data,
-                                                FALSE);
+                       setup_tx_uio(tx_pdu, tx_pdu->temp_data_len,
+                            tx_pdu->temp_data, FALSE);
                        send_pdu(tx_ccb, tx_pdu, CCBDISP_NOWAIT, PDUDISP_FREE);
                } else {
                        set_negotiated_parameters(tx_ccb);
@@ -1209,7 +1217,6 @@
        }
 
        setup_tx_uio(ppdu, 0, NULL, FALSE);
-
        send_pdu(ccb, ppdu, (wait) ? CCBDISP_WAIT : CCBDISP_FREE, PDUDISP_FREE);
 
        if (wait) {
@@ -1350,7 +1357,6 @@
                                sn, len, offs, totlen));
 
                setup_tx_uio(tx_pdu, len, tx_ccb->data_ptr + offs, FALSE);
-
                send_pdu(tx_ccb, tx_pdu, (totlen) ? CCBDISP_NOWAIT : disp, PDUDISP_FREE);
 



Home | Main Index | Thread Index | Old Index