possible fix to races in ami(4)
you cant test a variable and then sleep on it without blocking
interrupts, cos a completion could change the variables state between
those two actions.
could anyone test setting a hotspare on ami(4) while doing io?
dlg
Index: ami.c
===
RCS file: /cvs/src/sys/dev/ic/ami.c,v
retrieving revision 1.204
diff -u -p ami.c
--- ami.c 20 May 2010 00:55:17 - 1.204
+++ ami.c 31 May 2010 12:42:47 -
@@ -186,11 +186,8 @@ ami_remove_runq(struct ami_ccb *ccb)
splassert(IPL_BIO);
TAILQ_REMOVE(ccb-ccb_sc-sc_ccb_runq, ccb, ccb_link);
- if (TAILQ_EMPTY(ccb-ccb_sc-sc_ccb_runq)) {
- ccb-ccb_sc-sc_drained = 1;
- if (ccb-ccb_sc-sc_drainio)
- wakeup(ccb-ccb_sc);
- }
+ if (ccb-ccb_sc-sc_drainio TAILQ_EMPTY(ccb-ccb_sc-sc_ccb_runq))
+ wakeup(ccb-ccb_sc);
}
void
@@ -198,7 +195,6 @@ ami_insert_runq(struct ami_ccb *ccb)
{
splassert(IPL_BIO);
- ccb-ccb_sc-sc_drained = 0;
TAILQ_INSERT_TAIL(ccb-ccb_sc-sc_ccb_runq, ccb, ccb_link);
}
@@ -539,7 +535,6 @@ ami_attach(struct ami_softc *sc)
/* error already printed */
goto free_mbox;
}
- sc-sc_drained = 1;
/* hack for hp netraid version encoding */
if ('A' = sc-sc_fwver[2] sc-sc_fwver[2] = 'Z'
@@ -1016,7 +1011,6 @@ ami_runqueue(struct ami_softc *sc)
while ((ccb = TAILQ_FIRST(sc-sc_ccb_preq)) != NULL) {
if (sc-sc_exec(sc, ccb-ccb_cmd) != 0) {
- /* this is now raceable too with other incoming io */
timeout_add(sc-sc_run_tmo, 1);
break;
}
@@ -1895,10 +1889,8 @@ ami_mgmt(struct ami_softc *sc, u_int8_t opcode, u_int8
goto err;
}
ccb-ccb_done = ami_done_ioctl;
- } else {
+ } else
ccb = sc-sc_mgmtccb;
- ccb-ccb_done = ami_done_dummy;
- }
if (size) {
if ((am = ami_allocmem(sc, size)) == NULL) {
@@ -1930,22 +1922,29 @@ ami_mgmt(struct ami_softc *sc, u_int8_t opcode, u_int8
if (opcode != AMI_CHSTATE) {
ami_start(sc, ccb);
+ s = splbio();
while (ccb-ccb_state != AMI_CCB_READY)
tsleep(ccb, PRIBIO,ami_mgmt, 0);
+ splx(s);
} else {
/* change state must be run with id 0xfe and MUST be polled */
+ s = splbio();
sc-sc_drainio = 1;
- while (sc-sc_drained != 1)
+ while (!TAILQ_EMPTY(sc-sc_ccb_runq)) {
if (tsleep(sc, PRIBIO, ami_mgmt_drain, hz * 60) ==
EWOULDBLOCK) {
printf(%s: drain io timeout\n, DEVNAME(sc));
ccb-ccb_flags |= AMI_CCB_F_ERR;
goto restartio;
}
- ami_poll(sc, ccb);
+ }
+
+ error = sc-sc_poll(sc, ccb-ccb_cmd);
+ if (error == -1)
+ ccb-ccb_flags |= AMI_CCB_F_ERR;
+
restartio:
/* restart io */
- s = splbio();
sc-sc_drainio = 0;
ami_runqueue(sc);
splx(s);
@@ -1966,7 +1965,6 @@ memerr:
} else {
ccb-ccb_flags = 0;
ccb-ccb_state = AMI_CCB_FREE;
- ccb-ccb_done = NULL;
}
err:
Index: amivar.h
===
RCS file: /cvs/src/sys/dev/ic/amivar.h,v
retrieving revision 1.54
diff -u -p amivar.h
--- amivar.h28 Oct 2008 11:43:10 - 1.54
+++ amivar.h31 May 2010 12:42:47 -
@@ -149,7 +149,6 @@ struct ami_softc {
charsc_plist[AMI_BIG_MAX_PDRIVES];
struct ami_ccb *sc_mgmtccb;
- int sc_drained;
int sc_drainio;
u_int8_tsc_drvinscnt;
};
Re: possible fix to races in ami(4)
And make sure the hotspare kicks in. You HAVE to create the hotspare
under heavy io.
On Tue, Jun 01, 2010 at 11:09:53AM +1000, David Gwynne wrote:
you cant test a variable and then sleep on it without blocking
interrupts, cos a completion could change the variables state between
those two actions.
could anyone test setting a hotspare on ami(4) while doing io?
dlg
Index: ami.c
===
RCS file: /cvs/src/sys/dev/ic/ami.c,v
retrieving revision 1.204
diff -u -p ami.c
--- ami.c 20 May 2010 00:55:17 - 1.204
+++ ami.c 31 May 2010 12:42:47 -
@@ -186,11 +186,8 @@ ami_remove_runq(struct ami_ccb *ccb)
splassert(IPL_BIO);
TAILQ_REMOVE(ccb-ccb_sc-sc_ccb_runq, ccb, ccb_link);
- if (TAILQ_EMPTY(ccb-ccb_sc-sc_ccb_runq)) {
- ccb-ccb_sc-sc_drained = 1;
- if (ccb-ccb_sc-sc_drainio)
- wakeup(ccb-ccb_sc);
- }
+ if (ccb-ccb_sc-sc_drainio TAILQ_EMPTY(ccb-ccb_sc-sc_ccb_runq))
+ wakeup(ccb-ccb_sc);
}
void
@@ -198,7 +195,6 @@ ami_insert_runq(struct ami_ccb *ccb)
{
splassert(IPL_BIO);
- ccb-ccb_sc-sc_drained = 0;
TAILQ_INSERT_TAIL(ccb-ccb_sc-sc_ccb_runq, ccb, ccb_link);
}
@@ -539,7 +535,6 @@ ami_attach(struct ami_softc *sc)
/* error already printed */
goto free_mbox;
}
- sc-sc_drained = 1;
/* hack for hp netraid version encoding */
if ('A' = sc-sc_fwver[2] sc-sc_fwver[2] = 'Z'
@@ -1016,7 +1011,6 @@ ami_runqueue(struct ami_softc *sc)
while ((ccb = TAILQ_FIRST(sc-sc_ccb_preq)) != NULL) {
if (sc-sc_exec(sc, ccb-ccb_cmd) != 0) {
- /* this is now raceable too with other incoming io */
timeout_add(sc-sc_run_tmo, 1);
break;
}
@@ -1895,10 +1889,8 @@ ami_mgmt(struct ami_softc *sc, u_int8_t opcode, u_int8
goto err;
}
ccb-ccb_done = ami_done_ioctl;
- } else {
+ } else
ccb = sc-sc_mgmtccb;
- ccb-ccb_done = ami_done_dummy;
- }
if (size) {
if ((am = ami_allocmem(sc, size)) == NULL) {
@@ -1930,22 +1922,29 @@ ami_mgmt(struct ami_softc *sc, u_int8_t opcode, u_int8
if (opcode != AMI_CHSTATE) {
ami_start(sc, ccb);
+ s = splbio();
while (ccb-ccb_state != AMI_CCB_READY)
tsleep(ccb, PRIBIO,ami_mgmt, 0);
+ splx(s);
} else {
/* change state must be run with id 0xfe and MUST be polled */
+ s = splbio();
sc-sc_drainio = 1;
- while (sc-sc_drained != 1)
+ while (!TAILQ_EMPTY(sc-sc_ccb_runq)) {
if (tsleep(sc, PRIBIO, ami_mgmt_drain, hz * 60) ==
EWOULDBLOCK) {
printf(%s: drain io timeout\n, DEVNAME(sc));
ccb-ccb_flags |= AMI_CCB_F_ERR;
goto restartio;
}
- ami_poll(sc, ccb);
+ }
+
+ error = sc-sc_poll(sc, ccb-ccb_cmd);
+ if (error == -1)
+ ccb-ccb_flags |= AMI_CCB_F_ERR;
+
restartio:
/* restart io */
- s = splbio();
sc-sc_drainio = 0;
ami_runqueue(sc);
splx(s);
@@ -1966,7 +1965,6 @@ memerr:
} else {
ccb-ccb_flags = 0;
ccb-ccb_state = AMI_CCB_FREE;
- ccb-ccb_done = NULL;
}
err:
Index: amivar.h
===
RCS file: /cvs/src/sys/dev/ic/amivar.h,v
retrieving revision 1.54
diff -u -p amivar.h
--- amivar.h 28 Oct 2008 11:43:10 - 1.54
+++ amivar.h 31 May 2010 12:42:47 -
@@ -149,7 +149,6 @@ struct ami_softc {
charsc_plist[AMI_BIG_MAX_PDRIVES];
struct ami_ccb *sc_mgmtccb;
- int sc_drained;
int sc_drainio;
u_int8_tsc_drvinscnt;
};
