NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/39297: mfi calls tsleep() from mfi_intr()
The following reply was made to PR kern/39297; it has been noted by GNATS.
From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: kern-bug-people%NetBSD.org@localhost, gnats-admin%NetBSD.org@localhost,
netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/39297: mfi calls tsleep() from mfi_intr()
Date: Sat, 18 Oct 2008 22:27:32 +0200
--Dxnq1zWXvFF0Q93v
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Tue, Aug 05, 2008 at 05:25:00PM +0000, oster%NetBSD.org@localhost wrote:
> Running 4.99.71 (and some revisions earlier) on a machine with
> using the mfi will result in the machine eventually locking up. Breaking
> into ddb yields the following:
>
> login: fatal breakpoint trap in supervisor mode
> trap type 1 code 0 rip ffffffff804dba45 cs 8 rflags 202 cr2
> ffff8000720a8000 cp
> l 8 rsp ffff800062c4b7f8
> Stopped in pid 0.2 (system) at netbsd:breakpoint+0x5: leave
> db{0}> tr
> breakpoint() at netbsd:breakpoint+0x5
> comintr() at netbsd:comintr+0x53a
> Xintr_ioapic_edge6() at netbsd:Xintr_ioapic_edge6+0xef
> --- interrupt ---
> mutex_spin_retry() at netbsd:mutex_spin_retry+0x5a
> ltsleep() at netbsd:ltsleep+0xe5
> mfi_mgmt() at netbsd:mfi_mgmt+0xe1
> mfi_scsipi_request() at netbsd:mfi_scsipi_request+0x331
> scsipi_run_queue() at netbsd:scsipi_run_queue+0x16e
> mfi_intr() at netbsd:mfi_intr+0xc0
> intr_biglock_wrapper() at netbsd:intr_biglock_wrapper+0x1d
> Xintr_ioapic_level2() at netbsd:Xintr_ioapic_level2+0xf7
Hi, can you try the attached patch ? It should avoid tsleep in the
mfi_scsipi_request() path.
--
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
NetBSD: 26 ans d'experience feront toujours la difference
--
--Dxnq1zWXvFF0Q93v
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff
Index: mfi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/mfi.c,v
retrieving revision 1.18
diff -u -r1.18 mfi.c
--- mfi.c 24 Jun 2008 10:08:43 -0000 1.18
+++ mfi.c 18 Oct 2008 20:25:38 -0000
@@ -85,8 +85,10 @@
static int mfi_scsi_io(struct mfi_ccb *, struct scsipi_xfer *,
uint32_t, uint32_t);
static void mfi_scsi_xs_done(struct mfi_ccb *);
-static int mfi_mgmt(struct mfi_softc *, uint32_t, uint32_t,
- uint32_t, void *, uint8_t *);
+static int mfi_mgmt_internal(struct mfi_softc *,
+ uint32_t, uint32_t, uint32_t, void *, uint8_t *);
+static int mfi_mgmt(struct mfi_ccb *,struct scsipi_xfer *,
+ uint32_t, uint32_t, uint32_t, void *, uint8_t *);
static void mfi_mgmt_done(struct mfi_ccb *);
#if NBIO > 0
@@ -436,7 +438,7 @@
#endif
DNPRINTF(MFI_D_MISC, "%s: mfi_get_info\n", DEVNAME(sc));
- if (mfi_mgmt(sc, MR_DCMD_CTRL_GET_INFO, MFI_DATA_IN,
+ if (mfi_mgmt_internal(sc, MR_DCMD_CTRL_GET_INFO, MFI_DATA_IN,
sizeof(sc->sc_info), &sc->sc_info, NULL))
return 1;
@@ -701,10 +703,7 @@
memset(adapt, 0, sizeof(*adapt));
adapt->adapt_dev = &sc->sc_dev;
adapt->adapt_nchannels = 1;
- if (sc->sc_ld_cnt)
- adapt->adapt_openings = sc->sc_max_cmds / sc->sc_ld_cnt;
- else
- adapt->adapt_openings = sc->sc_max_cmds;
+ adapt->adapt_openings = sc->sc_max_cmds - 1;
adapt->adapt_max_periph = adapt->adapt_openings;
adapt->adapt_request = mfi_scsipi_request;
adapt->adapt_minphys = mfiminphys;
@@ -844,8 +843,10 @@
}
pcq->mpc_consumer = consumer;
- bus_dmamap_sync(sc->sc_dmat, MFIMEM_MAP(sc->sc_pcq), 0,
- sizeof(uint32_t) * sc->sc_max_cmds + sizeof(struct mfi_prod_cons),
+ bus_dmamap_sync(sc->sc_dmat, MFIMEM_MAP(sc->sc_pcq),
+ sizeof(uint32_t) * sc->sc_max_cmds +
+ offsetof(struct mfi_prod_cons, mpc_consumer),
+ sizeof(pcq->mpc_consumer),
BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
return claimed;
@@ -1073,19 +1074,13 @@
break;
case SCSI_SYNCHRONIZE_CACHE_10:
- mfi_put_ccb(ccb); /* we don't need this */
-
mbox[0] = MR_FLUSH_CTRL_CACHE | MR_FLUSH_DISK_CACHE;
- if (mfi_mgmt(sc, MR_DCMD_CTRL_CACHE_FLUSH, MFI_DATA_NONE,
- 0, NULL, mbox))
+ if (mfi_mgmt(ccb, xs,
+ MR_DCMD_CTRL_CACHE_FLUSH, MFI_DATA_NONE, 0, NULL, mbox)) {
+ mfi_put_ccb(ccb);
goto stuffup;
- xs->error = XS_NOERROR;
- xs->status = SCSI_OK;
- xs->resid = 0;
- scsipi_done(xs);
- splx(s);
- return;
- /* NOTREACHED */
+ }
+ break;
/* hand it of to the firmware and let it deal with it */
case SCSI_TEST_UNIT_READY:
@@ -1211,17 +1206,45 @@
}
static int
-mfi_mgmt(struct mfi_softc *sc, uint32_t opc, uint32_t dir, uint32_t len,
- void *buf, uint8_t *mbox)
-{
+mfi_mgmt_internal(struct mfi_softc *sc, uint32_t opc, uint32_t dir,
+ uint32_t len, void *buf, uint8_t *mbox) {
struct mfi_ccb *ccb;
- struct mfi_dcmd_frame *dcmd;
int rv = 1;
- DNPRINTF(MFI_D_MISC, "%s: mfi_mgmt %#x\n", DEVNAME(sc), opc);
-
if ((ccb = mfi_get_ccb(sc)) == NULL)
return rv;
+ rv = mfi_mgmt(ccb, NULL, opc, dir, len, buf, mbox);
+ if (rv)
+ return rv;
+
+ if (cold) {
+ if (mfi_poll(ccb))
+ goto done;
+ } else {
+ mfi_post(sc, ccb);
+
+ DNPRINTF(MFI_D_MISC, "%s: mfi_mgmt_internal sleeping\n",
+ DEVNAME(sc));
+ while (ccb->ccb_state != MFI_CCB_DONE)
+ tsleep(ccb, PRIBIO, "mfi_mgmt", 0);
+
+ if (ccb->ccb_flags & MFI_CCB_F_ERR)
+ goto done;
+ }
+ rv = 0;
+
+done:
+ mfi_put_ccb(ccb);
+ return rv;
+}
+
+static int
+mfi_mgmt(struct mfi_ccb *ccb, struct scsipi_xfer *xs,
+ uint32_t opc, uint32_t dir, uint32_t len, void *buf, uint8_t *mbox)
+{
+ struct mfi_dcmd_frame *dcmd;
+
+ DNPRINTF(MFI_D_MISC, "%s: mfi_mgmt %#x\n", DEVNAME(ccb->ccb_sc), opc);
dcmd = &ccb->ccb_frame->mfr_dcmd;
memset(dcmd->mdf_mbox, 0, MFI_MBOX_SIZE);
@@ -1231,6 +1254,7 @@
dcmd->mdf_opcode = opc;
dcmd->mdf_header.mfh_data_len = 0;
ccb->ccb_direction = dir;
+ ccb->ccb_xs = xs;
ccb->ccb_done = mfi_mgmt_done;
ccb->ccb_frame_size = MFI_DCMD_FRAME_SIZE;
@@ -1246,33 +1270,15 @@
ccb->ccb_sgl = &dcmd->mdf_sgl;
if (mfi_create_sgl(ccb, BUS_DMA_WAITOK))
- goto done;
- }
-
- if (cold) {
- if (mfi_poll(ccb))
- goto done;
- } else {
- mfi_post(sc, ccb);
-
- DNPRINTF(MFI_D_MISC, "%s: mfi_mgmt sleeping\n", DEVNAME(sc));
- while (ccb->ccb_state != MFI_CCB_DONE)
- tsleep(ccb, PRIBIO, "mfi_mgmt", 0);
-
- if (ccb->ccb_flags & MFI_CCB_F_ERR)
- goto done;
+ return 1;
}
-
- rv = 0;
-
-done:
- mfi_put_ccb(ccb);
- return rv;
+ return 0;
}
static void
mfi_mgmt_done(struct mfi_ccb *ccb)
{
+ struct scsipi_xfer *xs = ccb->ccb_xs;
struct mfi_softc *sc = ccb->ccb_sc;
struct mfi_frame_header *hdr = &ccb->ccb_frame->mfr_header;
@@ -1294,8 +1300,18 @@
ccb->ccb_flags |= MFI_CCB_F_ERR;
ccb->ccb_state = MFI_CCB_DONE;
-
- wakeup(ccb);
+ if (xs) {
+ if (hdr->mfh_cmd_status != MFI_STAT_OK) {
+ xs->error = XS_DRIVER_STUFFUP;
+ } else {
+ xs->error = XS_NOERROR;
+ xs->status = SCSI_OK;
+ xs->resid = 0;
+ }
+ mfi_put_ccb(ccb);
+ scsipi_done(xs);
+ } else
+ wakeup(ccb);
}
#if NBIO > 0
@@ -1365,7 +1381,8 @@
/* get figures */
cfg = malloc(sizeof *cfg, M_DEVBUF, M_WAITOK);
- if (mfi_mgmt(sc, MD_DCMD_CONF_GET, MFI_DATA_IN, sizeof *cfg, cfg, NULL))
+ if (mfi_mgmt_internal(sc, MD_DCMD_CONF_GET, MFI_DATA_IN,
+ sizeof *cfg, cfg, NULL))
goto freeme;
strlcpy(bi->bi_dev, DEVNAME(sc), sizeof(bi->bi_dev));
@@ -1387,7 +1404,7 @@
DNPRINTF(MFI_D_IOCTL, "%s: mfi_ioctl_vol %#x\n",
DEVNAME(sc), bv->bv_volid);
- if (mfi_mgmt(sc, MR_DCMD_LD_GET_LIST, MFI_DATA_IN,
+ if (mfi_mgmt_internal(sc, MR_DCMD_LD_GET_LIST, MFI_DATA_IN,
sizeof(sc->sc_ld_list), &sc->sc_ld_list, NULL))
goto done;
@@ -1396,7 +1413,7 @@
DNPRINTF(MFI_D_IOCTL, "%s: mfi_ioctl_vol target %#x\n",
DEVNAME(sc), mbox[0]);
- if (mfi_mgmt(sc, MR_DCMD_LD_GET_INFO, MFI_DATA_IN,
+ if (mfi_mgmt_internal(sc, MR_DCMD_LD_GET_INFO, MFI_DATA_IN,
sizeof(sc->sc_ld_details), &sc->sc_ld_details, mbox))
goto done;
@@ -1488,7 +1505,8 @@
/* send single element command to retrieve size for full structure */
cfg = malloc(sizeof *cfg, M_DEVBUF, M_WAITOK);
- if (mfi_mgmt(sc, MD_DCMD_CONF_GET, MFI_DATA_IN, sizeof *cfg, cfg, NULL))
+ if (mfi_mgmt_internal(sc, MD_DCMD_CONF_GET, MFI_DATA_IN,
+ sizeof *cfg, cfg, NULL))
goto freeme;
size = cfg->mfc_size;
@@ -1496,7 +1514,8 @@
/* memory for read config */
cfg = malloc(size, M_DEVBUF, M_WAITOK|M_ZERO);
- if (mfi_mgmt(sc, MD_DCMD_CONF_GET, MFI_DATA_IN, size, cfg, NULL))
+ if (mfi_mgmt_internal(sc, MD_DCMD_CONF_GET, MFI_DATA_IN,
+ size, cfg, NULL))
goto freeme;
ar = cfg->mfc_array;
@@ -1560,7 +1579,7 @@
/* get the remaining fields */
*((uint16_t *)&mbox) = ar[arr].pd[disk].mar_pd.mfp_id;
memset(pd, 0, sizeof(*pd));
- if (mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
+ if (mfi_mgmt_internal(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
sizeof *pd, pd, mbox))
goto freeme;
@@ -1620,7 +1639,7 @@
return EINVAL;
}
- if (mfi_mgmt(sc, opc, dir, sizeof(ret), &ret, NULL))
+ if (mfi_mgmt_internal(sc, opc, dir, sizeof(ret), &ret, NULL))
rv = EINVAL;
else
if (ba->ba_opcode == BIOC_GASTATUS)
@@ -1648,7 +1667,7 @@
pd = malloc(MFI_PD_LIST_SIZE, M_DEVBUF, M_WAITOK);
- if (mfi_mgmt(sc, MR_DCMD_PD_GET_LIST, MFI_DATA_IN,
+ if (mfi_mgmt_internal(sc, MR_DCMD_PD_GET_LIST, MFI_DATA_IN,
MFI_PD_LIST_SIZE, pd, NULL))
goto done;
@@ -1683,7 +1702,7 @@
}
- if (mfi_mgmt(sc, cmd, MFI_DATA_NONE, 0, NULL, mbox))
+ if (mfi_mgmt_internal(sc, cmd, MFI_DATA_NONE, 0, NULL, mbox))
goto done;
rv = 0;
@@ -1705,7 +1724,7 @@
pd = malloc(MFI_PD_LIST_SIZE, M_DEVBUF, M_WAITOK);
- if (mfi_mgmt(sc, MR_DCMD_PD_GET_LIST, MFI_DATA_IN,
+ if (mfi_mgmt_internal(sc, MR_DCMD_PD_GET_LIST, MFI_DATA_IN,
MFI_PD_LIST_SIZE, pd, NULL))
goto done;
@@ -1750,7 +1769,8 @@
}
- if (mfi_mgmt(sc, MD_DCMD_PD_SET_STATE, MFI_DATA_NONE, 0, NULL, mbox))
+ if (mfi_mgmt_internal(sc, MD_DCMD_PD_SET_STATE, MFI_DATA_NONE,
+ 0, NULL, mbox))
goto done;
rv = 0;
@@ -1782,7 +1802,8 @@
/* send single element command to retrieve size for full structure */
cfg = malloc(sizeof *cfg, M_DEVBUF, M_WAITOK);
- if (mfi_mgmt(sc, MD_DCMD_CONF_GET, MFI_DATA_IN, sizeof *cfg, cfg, NULL))
+ if (mfi_mgmt_internal(sc, MD_DCMD_CONF_GET, MFI_DATA_IN,
+ sizeof *cfg, cfg, NULL))
goto freeme;
size = cfg->mfc_size;
@@ -1790,7 +1811,8 @@
/* memory for read config */
cfg = malloc(size, M_DEVBUF, M_WAITOK|M_ZERO);
- if (mfi_mgmt(sc, MD_DCMD_CONF_GET, MFI_DATA_IN, size, cfg, NULL))
+ if (mfi_mgmt_internal(sc, MD_DCMD_CONF_GET, MFI_DATA_IN,
+ size, cfg, NULL))
goto freeme;
/* calculate offset to hs structure */
@@ -1815,7 +1837,7 @@
/* get pd fields */
memset(mbox, 0, sizeof mbox);
*((uint16_t *)&mbox) = hs[i].mhs_pd.mfp_id;
- if (mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
+ if (mfi_mgmt_internal(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
sizeof *pd, pd, mbox)) {
DNPRINTF(MFI_D_IOCTL, "%s: mfi_vol_hs illegal PD\n",
DEVNAME(sc));
--Dxnq1zWXvFF0Q93v--
Home |
Main Index |
Thread Index |
Old Index