fa.openbsd.tech
[Top] [All Lists]

sdmmc diff, please test

Subject: sdmmc diff, please test
From: "Bret S. Lambert"
Date: Mon, 09 Mar 2009 12:24:19 UTC
Newsgroups: fa.openbsd.tech

Trying to clean up the locking in the kernel (i.e., removing recursion
in locking schemes), I've come up with the following diff to the sdmmc
code. However, lacking any hardware of my own, I need someone to give
it a run (especially on an x40).

If, as is hopefully no longer likely, you encounter the dreaded
"locking against myself" panic, please send the function call list
obtained with `trace' from the ddb prompt.

Reports of success are, of course, just as necessary as reports of failure.

I know that Mondays are probably bad to ask for testing of diffs
as everyone shakes off their weekend hangovers and stumbles through
the workday, but I'm somewhat anxious to get this (back) in.

Any questions or comments, please send them my way.

Thanks

- Bert

Index: dev/sdmmc//sdmmc.c
===================================================================
RCS file: /cvs/openbsd//src/sys/dev/sdmmc/sdmmc.c,v
retrieving revision 1.19
diff -u -p -r1.19 sdmmc.c
--- dev/sdmmc//sdmmc.c  20 Feb 2009 19:16:35 -0000      1.19
+++ dev/sdmmc//sdmmc.c  8 Mar 2009 11:24:34 -0000
@@ -110,7 +110,7 @@ sdmmc_attach(struct device *parent, stru
        TAILQ_INIT(&sc->sc_intrq);
        sdmmc_init_task(&sc->sc_discover_task, sdmmc_discover_task, sc);
        sdmmc_init_task(&sc->sc_intr_task, sdmmc_intr_task, sc);
-       lockinit(&sc->sc_lock, PRIBIO, DEVNAME(sc), 0, LK_CANRECURSE);
+       lockinit(&sc->sc_lock, PRIBIO, DEVNAME(sc), 0, 0);
 
 #ifdef SDMMC_IOCTL
        if (bio_register(self, sdmmc_ioctl) != 0)
@@ -176,8 +176,11 @@ sdmmc_task_thread(void *arg)
        }
        splx(s);
 
-       if (ISSET(sc->sc_flags, SMF_CARD_PRESENT))
+       if (ISSET(sc->sc_flags, SMF_CARD_PRESENT)) {
+               SDMMC_LOCK(sc);
                sdmmc_card_detach(sc, DETACH_FORCE);
+               SDMMC_UNLOCK(sc);
+       }
 
        sc->sc_task_thread = NULL;
        wakeup(sc);
@@ -235,7 +238,9 @@ sdmmc_discover_task(void *arg)
        } else {
                if (ISSET(sc->sc_flags, SMF_CARD_PRESENT)) {
                        CLR(sc->sc_flags, SMF_CARD_PRESENT);
+                       SDMMC_LOCK(sc);
                        sdmmc_card_detach(sc, DETACH_FORCE);
+                       SDMMC_UNLOCK(sc);
                }
        }
 }
@@ -301,6 +306,8 @@ sdmmc_card_detach(struct sdmmc_softc *sc
 {
        struct sdmmc_function *sf, *sfnext;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        DPRINTF(1,("%s: detach card\n", DEVNAME(sc)));
 
        if (ISSET(sc->sc_flags, SMF_CARD_ATTACHED)) {
@@ -334,6 +341,8 @@ sdmmc_enable(struct sdmmc_softc *sc)
        u_int32_t host_ocr;
        int error;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        /*
         * Calculate the equivalent of the card OCR from the host
         * capabilities and select the maximum supported bus voltage.
@@ -374,6 +383,7 @@ sdmmc_enable(struct sdmmc_softc *sc)
  err:
        if (error != 0)
                sdmmc_disable(sc);
+
        return error;
 }
 
@@ -382,6 +392,8 @@ sdmmc_disable(struct sdmmc_softc *sc)
 {
        /* XXX complete commands if card is still present. */
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        /* Make sure no card is still selected. */
        (void)sdmmc_select_card(sc, NULL);
 
@@ -399,6 +411,8 @@ sdmmc_set_bus_power(struct sdmmc_softc *
 {
        u_int32_t bit;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        /* Mask off unsupported voltage levels and select the lowest. */
        DPRINTF(1,("%s: host_ocr=%x ", DEVNAME(sc), host_ocr));
        host_ocr &= card_ocr;
@@ -444,6 +458,9 @@ sdmmc_function_free(struct sdmmc_functio
 int
 sdmmc_scan(struct sdmmc_softc *sc)
 {
+
+       SDMMC_ASSERT_LOCKED(sc);
+
        /* Scan for I/O functions. */
        if (ISSET(sc->sc_flags, SMF_IO_MODE))
                sdmmc_io_scan(sc);
@@ -469,6 +486,8 @@ sdmmc_init(struct sdmmc_softc *sc)
 {
        struct sdmmc_function *sf;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        /* Initialize all identified card functions. */
        SIMPLEQ_FOREACH(sf, &sc->sf_head, sf_list) {
                if (ISSET(sc->sc_flags, SMF_IO_MODE) &&
@@ -506,7 +525,7 @@ sdmmc_app_command(struct sdmmc_softc *sc
        struct sdmmc_command acmd;
        int error;
 
-       SDMMC_LOCK(sc);
+       SDMMC_ASSERT_LOCKED(sc);
 
        bzero(&acmd, sizeof acmd);
        acmd.c_opcode = MMC_APP_CMD;
@@ -515,18 +534,15 @@ sdmmc_app_command(struct sdmmc_softc *sc
 
        error = sdmmc_mmc_command(sc, &acmd);
        if (error != 0) {
-               SDMMC_UNLOCK(sc);
                return error;
        }
 
        if (!ISSET(MMC_R1(acmd.c_resp), MMC_R1_APP_CMD)) {
                /* Card does not support application commands. */
-               SDMMC_UNLOCK(sc);
                return ENODEV;
        }
 
        error = sdmmc_mmc_command(sc, cmd);
-       SDMMC_UNLOCK(sc);
        return error;
 }
 
@@ -540,7 +556,7 @@ sdmmc_mmc_command(struct sdmmc_softc *sc
 {
        int error;
 
-       SDMMC_LOCK(sc);
+       SDMMC_ASSERT_LOCKED(sc);
 
        sdmmc_chip_exec_command(sc->sct, sc->sch, cmd);
 
@@ -551,7 +567,6 @@ sdmmc_mmc_command(struct sdmmc_softc *sc
        error = cmd->c_error;
        wakeup(cmd);
 
-       SDMMC_UNLOCK(sc);
        return error;
 }
 
@@ -563,6 +578,8 @@ sdmmc_go_idle_state(struct sdmmc_softc *
 {
        struct sdmmc_command cmd;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        bzero(&cmd, sizeof cmd);
        cmd.c_opcode = MMC_GO_IDLE_STATE;
        cmd.c_flags = SCF_CMD_BC | SCF_RSP_R0;
@@ -580,6 +597,8 @@ sdmmc_send_if_cond(struct sdmmc_softc *s
        uint8_t pat = 0x23;     /* any pattern will do here */
        uint8_t res;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        bzero(&cmd, sizeof cmd);
 
        cmd.c_opcode = SD_SEND_IF_COND;
@@ -605,6 +624,8 @@ sdmmc_set_relative_addr(struct sdmmc_sof
 {
        struct sdmmc_command cmd;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        bzero(&cmd, sizeof cmd);
 
        if (ISSET(sc->sc_flags, SMF_SD_MODE)) {
@@ -661,6 +682,8 @@ sdmmc_select_card(struct sdmmc_softc *sc
        struct sdmmc_command cmd;
        int error;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        if (sc->sc_card == sf || (sf && sc->sc_card &&
            sc->sc_card->rca == sf->rca)) {
                sc->sc_card = sf;
@@ -727,10 +750,12 @@ sdmmc_ioctl(struct device *self, u_long 
                        cmd.c_datalen = ucmd->c_datalen;
                }
 
+               SDMMC_LOCK(sc);
                if (request == SDIOCEXECMMC)
                        error = sdmmc_mmc_command(sc, &cmd);
                else
                        error = sdmmc_app_command(sc, &cmd);
+               SDMMC_UNLOCK(sc);
                if (error && !cmd.c_error)
                        cmd.c_error = error;
 
@@ -758,6 +783,8 @@ void
 sdmmc_dump_command(struct sdmmc_softc *sc, struct sdmmc_command *cmd)
 {
        int i;
+
+       SDMMC_ASSERT_LOCKED(sc);
 
        DPRINTF(1,("%s: cmd %u arg=%#x data=%#x dlen=%d flags=%#x "
            "proc=\"%s\" (error %d)\n", DEVNAME(sc), cmd->c_opcode,
Index: dev/sdmmc//sdmmc_io.c
===================================================================
RCS file: /cvs/openbsd//src/sys/dev/sdmmc/sdmmc_io.c,v
retrieving revision 1.12
diff -u -p -r1.12 sdmmc_io.c
--- dev/sdmmc//sdmmc_io.c       2 Dec 2008 23:49:54 -0000       1.12
+++ dev/sdmmc//sdmmc_io.c       7 Mar 2009 23:50:36 -0000
@@ -71,6 +71,8 @@ sdmmc_io_enable(struct sdmmc_softc *sc)
        u_int32_t host_ocr;
        u_int32_t card_ocr;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        /* Set host mode to SD "combo" card. */
        SET(sc->sc_flags, SMF_SD_MODE|SMF_IO_MODE|SMF_MEM_MODE);
 
@@ -133,6 +135,8 @@ sdmmc_io_scan(struct sdmmc_softc *sc)
        struct sdmmc_function *sf0, *sf;
        int i;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        sf0 = sdmmc_function_alloc(sc);
        sf0->number = 0;
        if (sdmmc_set_relative_addr(sc, sf0) != 0) {
@@ -166,6 +170,8 @@ sdmmc_io_scan(struct sdmmc_softc *sc)
 int
 sdmmc_io_init(struct sdmmc_softc *sc, struct sdmmc_function *sf)
 {
+       SDMMC_ASSERT_LOCKED(sc);
+
        if (sf->number == 0) {
                sdmmc_io_write_1(sf, SD_IO_CCCR_BUS_WIDTH,
                    CCCR_BUS_WIDTH_1);
@@ -254,6 +260,8 @@ sdmmc_io_attach(struct sdmmc_softc *sc)
        struct sdmmc_function *sf;
        struct sdmmc_attach_args saa;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        SIMPLEQ_FOREACH(sf, &sc->sf_head, sf_list) {
                if (sf->number < 1)
                        continue;
@@ -325,6 +333,8 @@ sdmmc_io_detach(struct sdmmc_softc *sc)
 {
        struct sdmmc_function *sf;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        SIMPLEQ_FOREACH(sf, &sc->sf_head, sf_list) {
                if (sf->child != NULL) {
                        config_detach(sf->child, DETACH_FORCE);
@@ -534,7 +544,7 @@ sdmmc_io_send_op_cond(struct sdmmc_softc
        int error;
        int i;
 
-       SDMMC_LOCK(sc);
+       SDMMC_ASSERT_LOCKED(sc);
 
        /*
         * If we change the OCR value, retry the command until the OCR
@@ -559,7 +569,6 @@ sdmmc_io_send_op_cond(struct sdmmc_softc
        if (error == 0 && ocrp != NULL)
                *ocrp = MMC_R4(cmd.c_resp);
 
-       SDMMC_UNLOCK(sc);
        return error;
 }
 
Index: dev/sdmmc//sdmmc_mem.c
===================================================================
RCS file: /cvs/openbsd//src/sys/dev/sdmmc/sdmmc_mem.c,v
retrieving revision 1.11
diff -u -p -r1.11 sdmmc_mem.c
--- dev/sdmmc//sdmmc_mem.c      20 Feb 2009 19:16:35 -0000      1.11
+++ dev/sdmmc//sdmmc_mem.c      7 Mar 2009 23:41:34 -0000
@@ -51,6 +51,8 @@ sdmmc_mem_enable(struct sdmmc_softc *sc)
        u_int32_t host_ocr;
        u_int32_t card_ocr;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        /* Set host mode to SD "combo" card or SD memory-only. */
        SET(sc->sc_flags, SMF_SD_MODE|SMF_MEM_MODE);
 
@@ -117,6 +119,8 @@ sdmmc_mem_scan(struct sdmmc_softc *sc)
        int error;
        int i;
 
+       SDMMC_ASSERT_LOCKED(sc);
+
        /*
         * CMD2 is a broadcast command understood by SD cards and MMC
         * cards.  All cards begin to respond to the command, but back
@@ -324,11 +328,11 @@ sdmmc_mem_init(struct sdmmc_softc *sc, s
 {
        int error = 0;
 
-       SDMMC_LOCK(sc);
+       SDMMC_ASSERT_LOCKED(sc);
+
        if (sdmmc_select_card(sc, sf) != 0 ||
            sdmmc_mem_set_blocklen(sc, sf) != 0)
                error = 1;
-       SDMMC_UNLOCK(sc);
        return error;
 }
 
@@ -343,7 +347,7 @@ sdmmc_mem_send_op_cond(struct sdmmc_soft
        int error;
        int i;
 
-       SDMMC_LOCK(sc);
+       SDMMC_ASSERT_LOCKED(sc);
 
        /*
         * If we change the OCR value, retry the command until the OCR
@@ -373,7 +377,6 @@ sdmmc_mem_send_op_cond(struct sdmmc_soft
        if (error == 0 && ocrp != NULL)
                *ocrp = MMC_R3(cmd.c_resp);
 
-       SDMMC_UNLOCK(sc);
        return error;
 }
 
@@ -385,6 +388,8 @@ int
 sdmmc_mem_set_blocklen(struct sdmmc_softc *sc, struct sdmmc_function *sf)
 {
        struct sdmmc_command cmd;
+
+       SDMMC_ASSERT_LOCKED(sc);
 
        bzero(&cmd, sizeof cmd);
        cmd.c_opcode = MMC_SET_BLOCKLEN;
Index: dev/sdmmc//sdmmc_scsi.c
===================================================================
RCS file: /cvs/openbsd//src/sys/dev/sdmmc/sdmmc_scsi.c,v
retrieving revision 1.15
diff -u -p -r1.15 sdmmc_scsi.c
--- dev/sdmmc//sdmmc_scsi.c     20 Feb 2009 19:16:35 -0000      1.15
+++ dev/sdmmc//sdmmc_scsi.c     7 Mar 2009 23:51:12 -0000
@@ -99,10 +99,11 @@ sdmmc_scsi_attach(struct sdmmc_softc *sc
        struct sdmmc_scsi_softc *scbus;
        struct sdmmc_function *sf;
 
-       scbus = (struct sdmmc_scsi_softc *)malloc(sizeof *scbus,
-           M_DEVBUF, M_WAITOK | M_ZERO);
+       SDMMC_ASSERT_LOCKED(sc);
 
-       scbus->sc_tgt = (struct sdmmc_scsi_target 
*)malloc(sizeof(*scbus->sc_tgt) *
+       scbus = malloc(sizeof(*scbus), M_DEVBUF, M_WAITOK | M_ZERO);
+
+       scbus->sc_tgt = malloc(sizeof(*scbus->sc_tgt) *
            (SDMMC_SCSIID_MAX+1), M_DEVBUF, M_WAITOK | M_ZERO);
 
        /*
@@ -159,6 +160,8 @@ sdmmc_scsi_detach(struct sdmmc_softc *sc
        struct sdmmc_scsi_softc *scbus;
        struct sdmmc_ccb *ccb;
        int s;
+
+       SDMMC_ASSERT_LOCKED(sc);
 
        scbus = sc->sc_scsibus;
        if (scbus == NULL)
Index: dev/sdmmc//sdmmcvar.h
===================================================================
RCS file: /cvs/openbsd//src/sys/dev/sdmmc/sdmmcvar.h,v
retrieving revision 1.15
diff -u -p -r1.15 sdmmcvar.h
--- dev/sdmmc//sdmmcvar.h       20 Feb 2009 19:16:35 -0000      1.15
+++ dev/sdmmc//sdmmcvar.h       7 Mar 2009 22:14:15 -0000
@@ -191,6 +191,8 @@ struct sdmmc_attach_args {
 
 #define SDMMC_LOCK(sc)  lockmgr(&(sc)->sc_lock, LK_EXCLUSIVE, NULL)
 #define SDMMC_UNLOCK(sc) lockmgr(&(sc)->sc_lock, LK_RELEASE, NULL)
+#define        SDMMC_ASSERT_LOCKED(sc) \
+       KASSERT(lockstatus(&((sc))->sc_lock) == LK_EXCLUSIVE)
 
 void   sdmmc_add_task(struct sdmmc_softc *, struct sdmmc_task *);
 void   sdmmc_del_task(struct sdmmc_task *);


<Prev in Thread] Current Thread [Next in Thread>