Re: Request for testers - socket locking diff

[Iain Hibbert <plunky%rya-online.net@localhost>]

On Sat, 29 Mar 2008, Andrew Doran wrote:

> The below diff makes the socket code and one of the protocols (unix) MP
> safe. It needs clean-up and more testing but I believe that it works at
> least for IPv4 and Unix domain sockets.
>
> If anyone is interested in giving it a go, I'd love to hear whether or not
> it works - especially with Bluetooth and Appletalk since those are protocols
> that I don't have the setup for. Note that I haven't tried IPv6, IPsec or
> netbooting yet but I plan to soon.

for Bluetooth, the bthidev(4) and btsco(4) drivers in dev/bluetooth also
access the protocol KPI directly rather than go through the socket layer.
I think they can use bt_lock directly as below?  I'm building a kernel now
and will report back in a few days..

iain
(I have no MP machine however)

--- /usr/src/sys/dev/bluetooth/bthidev.c        2008-03-28 21:17:37.000000000 
+0000
+++ bthidev.c   2008-04-01 18:39:35.000000000 +0100
@@ -180,7 +180,7 @@
        struct hid_item h;
        const void *desc;
        int locs[BTHIDBUSCF_NLOCS];
-       int maxid, rep, s, dlen;
+       int maxid, rep, dlen;

        /*
         * Init softc
@@ -302,13 +302,13 @@
        /*
         * start bluetooth connections
         */
-       s = splsoftnet();
+       mutex_enter(bt_lock);
        if ((sc->sc_flags & BTHID_RECONNECT) == 0)
                bthidev_listen(sc);

        if (sc->sc_flags & BTHID_CONNECTING)
                bthidev_connect(sc);
-       splx(s);
+       mutex_exit(bt_lock);
 }

 static int
@@ -316,9 +316,8 @@
 {
        struct bthidev_softc *sc = device_private(self);
        struct bthidev *hidev;
-       int s;

-       s = splsoftnet();
+       mutex_enter(bt_lock);
        sc->sc_flags = 0;       /* disable reconnecting */

        /* release interrupt listen */
@@ -355,7 +354,7 @@

        callout_destroy(&sc->sc_reconnect);

-       splx(s);
+       mutex_exit(bt_lock);

        /* detach children */
        while ((hidev = LIST_FIRST(&sc->sc_list)) != NULL) {
@@ -396,9 +395,8 @@
 bthidev_timeout(void *arg)
 {
        struct bthidev_softc *sc = arg;
-       int s;

-       s = splsoftnet();
+       mutex_enter(bt_lock);
        callout_ack(&sc->sc_reconnect);

        switch (sc->sc_state) {
@@ -437,7 +435,7 @@
        default:
                break;
        }
-       splx(s);
+       mutex_exit(bt_lock);
 }

 /*
@@ -865,7 +863,7 @@
 {
        struct bthidev_softc *sc = device_private(hidev->sc_parent);
        struct mbuf *m;
-       int s, err;
+       int err;

        if (sc == NULL || sc->sc_state != BTHID_OPEN)
                return ENOTCONN;
@@ -896,9 +894,9 @@
        memcpy(mtod(m, uint8_t *) + 2, report, rlen);
        m->m_pkthdr.len = m->m_len = rlen + 2;

-       s = splsoftnet();
+       mutex_enter(bt_lock);
        err = l2cap_send(sc->sc_int, m);
-       splx(s);
+       mutex_exit(bt_lock);

        return err;
 }
--- /usr/src/sys/dev/bluetooth/btsco.c  2008-03-28 21:17:37.000000000 +0000
+++ btsco.c     2008-04-01 18:40:54.000000000 +0100
@@ -339,11 +339,10 @@
 btsco_detach(device_t self, int flags)
 {
        struct btsco_softc *sc = device_private(self);
-       int s;

        DPRINTF("sc=%p\n", sc);

-       s = splsoftnet();
+       mutex_enter(bt_lock);
        if (sc->sc_sco != NULL) {
                DPRINTF("sc_sco=%p\n", sc->sc_sco);
                sco_disconnect(sc->sc_sco, 0);
@@ -356,7 +355,7 @@
                sco_detach(&sc->sc_sco_l);
                sc->sc_sco_l = NULL;
        }
-       splx(s);
+       mutex_exit(bt_lock);

        if (sc->sc_audio != NULL) {
                DPRINTF("sc_audio=%p\n", sc->sc_audio);
@@ -557,7 +556,7 @@
 {
        struct sockaddr_bt sa;
        struct btsco_softc *sc = hdl;
-       int err, s, timo;
+       int err, timo;

        DPRINTF("%s flags 0x%x\n", sc->sc_name, flags);
        /* flags FREAD & FWRITE? */
@@ -565,7 +564,7 @@
        if (sc->sc_sco != NULL || sc->sc_sco_l != NULL)
                return EIO;

-       s = splsoftnet();
+       mutex_enter(bt_lock);

        memset(&sa, 0, sizeof(sa));
        sa.bt_len = sizeof(sa);
@@ -639,7 +638,7 @@
        }

 done:
-       splx(s);
+       mutex_exit(bt_lock);

        DPRINTF("done err=%d, sc_state=%d, sc_mtu=%d\n",
                        err, sc->sc_state, sc->sc_mtu);
@@ -650,11 +649,10 @@
 btsco_close(void *hdl)
 {
        struct btsco_softc *sc = hdl;
-       int s;

        DPRINTF("%s\n", sc->sc_name);

-       s = splsoftnet();
+       mutex_enter(bt_lock);
        if (sc->sc_sco != NULL) {
                sco_disconnect(sc->sc_sco, 0);
                sco_detach(&sc->sc_sco);
@@ -663,7 +661,7 @@
        if (sc->sc_sco_l != NULL) {
                sco_detach(&sc->sc_sco_l);
        }
-       splx(s);
+       mutex_exit(bt_lock);

        if (sc->sc_rx_mbuf != NULL) {
                m_freem(sc->sc_rx_mbuf);