Subject: Re: Draining after adjusting max openings -- not necessary?
To: Jason Thorpe <thorpej@wasabisystems.com>
From: Jason Thorpe <thorpej@wasabisystems.com>
List: tech-kern
Date: 06/12/2003 19:33:16
--Apple-Mail-3--145014401
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
charset=US-ASCII;
format=flowed
On Saturday, June 7, 2003, at 08:36 PM, Jason Thorpe wrote:
> Attached is a patch that addresses both issues. Anyone see any
> problems with these changes?
To follow-up, here is the patch I'm going to check in. I have verified
it works properly on an Intel SRCS14L S-ATA RAID controller (icp
driver), using the Intel/ICP-Vortex management tool (iirconfig, built
natively for NetBSD) to dynamically create and delete cache serivce
array drives while the system is running (which causes the # of
available command openings to change). Changes to do the array drive
rescan in the icp driver are forthcoming.
This patch does the following:
dev/ld.c:
* ldadjqparam() -- don't wait for commands to drain.
* ldbegindetach() -- ...instead, do it here, where it is actually
wanted/needed. Protect ourselves against spurious wakeups
while waiting for the drain to occur.
* ldenddetach() -- Don't flush the cache here. We can't reliably
do it, since, in all likelihood, the array controller no longer
knows about the underlying array volume. If the user wanted the
cache flushed, presumably they will unmount the file system (thus
closing the device, which also flushes the cache) before deleting
the array volume.
* lddone() -- fix sleep/wakeup protocol botch with draining.
dev/i2o/iopsp.c:
* iopsp_attach() -- set the initial number of openings to 1
so that...
* iopsp_adjqparam() -- the adjustment here works properly.
* iopsp_scsipi_request() -- remove curqd.
dev/i2o/iopspvar.h:
* replace sc_curqd with sc_openings.
-- Jason R. Thorpe <thorpej@wasabisystems.com>
--Apple-Mail-3--145014401
Content-Disposition: attachment;
filename=lddrain-patch
Content-Transfer-Encoding: 7bit
Content-Type: application/octet-stream;
x-unix-mode=0644;
name="lddrain-patch"
Index: ld.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ld.c,v
retrieving revision 1.23
diff -c -r1.23 ld.c
*** ld.c 2003/06/07 23:37:24 1.23
--- ld.c 2003/06/13 02:14:08
***************
*** 157,181 ****
int
ldadjqparam(struct ld_softc *sc, int max)
{
! int s, rv;
s = splbio();
sc->sc_maxqueuecnt = max;
- if (sc->sc_queuecnt > max) {
- sc->sc_flags |= LDF_DRAIN;
- rv = tsleep(&sc->sc_queuecnt, PRIBIO, "lddrn", 30 * hz);
- sc->sc_flags &= ~LDF_DRAIN;
- } else
- rv = 0;
splx(s);
! return (rv);
}
int
ldbegindetach(struct ld_softc *sc, int flags)
{
! int s, rv;
if ((sc->sc_flags & LDF_ENABLED) == 0)
return (0);
--- 157,175 ----
int
ldadjqparam(struct ld_softc *sc, int max)
{
! int s;
s = splbio();
sc->sc_maxqueuecnt = max;
splx(s);
! return (0);
}
int
ldbegindetach(struct ld_softc *sc, int flags)
{
! int s, rv = 0;
if ((sc->sc_flags & LDF_ENABLED) == 0)
return (0);
***************
*** 184,191 ****
return (EBUSY);
s = splbio();
sc->sc_flags |= LDF_DETACH;
! rv = ldadjqparam(sc, 0);
splx(s);
return (rv);
--- 178,191 ----
return (EBUSY);
s = splbio();
+ sc->sc_maxqueuecnt = 0;
sc->sc_flags |= LDF_DETACH;
! while (sc->sc_queuecnt > 0) {
! sc->sc_flags |= LDF_DRAIN;
! rv = tsleep(&sc->sc_queuecnt, PRIBIO, "lddrn", 0);
! if (rv)
! break;
! }
splx(s);
return (rv);
***************
*** 235,245 ****
--- 235,252 ----
rnd_detach_source(&sc->sc_rnd_source);
#endif
+ /*
+ * XXX We can't really flush the cache here, beceause the
+ * XXX device may already be non-existent from the controller's
+ * XXX perspective.
+ */
+ #if 0
/* Flush the device's cache. */
if (sc->sc_flush != NULL)
if ((*sc->sc_flush)(sc) != 0)
printf("%s: unable to flush cache\n",
sc->sc_dv.dv_xname);
+ #endif
}
/* ARGSUSED */
***************
*** 597,604 ****
biodone(bp);
if (--sc->sc_queuecnt <= sc->sc_maxqueuecnt) {
! if ((sc->sc_flags & LDF_DRAIN) != 0)
wakeup(&sc->sc_queuecnt);
ldstart(sc);
}
}
--- 604,613 ----
biodone(bp);
if (--sc->sc_queuecnt <= sc->sc_maxqueuecnt) {
! if ((sc->sc_flags & LDF_DRAIN) != 0) {
! sc->sc_flags &= ~LDF_DRAIN;
wakeup(&sc->sc_queuecnt);
+ }
ldstart(sc);
}
}
Index: i2o/iopsp.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2o/iopsp.c,v
retrieving revision 1.17
diff -c -r1.17 iopsp.c
*** i2o/iopsp.c 2002/12/06 11:22:25 1.17
--- i2o/iopsp.c 2003/06/13 02:14:10
***************
*** 179,184 ****
--- 179,186 ----
le32toh(param.p.sci.initiatorid));
#endif
+ sc->sc_openings = 1;
+
sc->sc_adapter.adapt_dev = &sc->sc_dv;
sc->sc_adapter.adapt_nchannels = 1;
sc->sc_adapter.adapt_openings = 1;
***************
*** 410,416 ****
struct iop_msg *im;
struct iop_softc *iop;
struct i2o_scsi_scb_exec *mf;
! int error, flags, tid, s;
u_int32_t mb[IOP_MAX_MSG_SIZE / sizeof(u_int32_t)];
sc = (void *)chan->chan_adapter->adapt_dev;
--- 412,418 ----
struct iop_msg *im;
struct iop_softc *iop;
struct i2o_scsi_scb_exec *mf;
! int error, flags, tid;
u_int32_t mb[IOP_MAX_MSG_SIZE / sizeof(u_int32_t)];
sc = (void *)chan->chan_adapter->adapt_dev;
***************
*** 496,509 ****
mf->flags |= I2O_SCB_FLAG_XFER_FROM_DEVICE;
}
- s = splbio();
- sc->sc_curqd++;
- splx(s);
-
if (iop_msg_post(iop, im, mb, xs->timeout)) {
- s = splbio();
- sc->sc_curqd--;
- splx(s);
if (xs->datalen != 0)
iop_msg_unmap(iop, im);
iop_msg_free(iop, im);
--- 498,504 ----
***************
*** 631,639 ****
iop_msg_unmap(iop, im);
iop_msg_free(iop, im);
- if (--sc->sc_curqd == sc->sc_adapter.adapt_openings)
- wakeup(&sc->sc_curqd);
-
scsipi_done(xs);
}
--- 626,631 ----
***************
*** 680,687 ****
sc = (struct iopsp_softc *)dv;
s = splbio();
! sc->sc_adapter.adapt_openings = mpi;
! if (mpi < sc->sc_curqd)
! tsleep(&sc->sc_curqd, PWAIT, "iopspdrn", 0);
splx(s);
}
--- 672,678 ----
sc = (struct iopsp_softc *)dv;
s = splbio();
! sc->sc_adapter.adapt_openings += mpi - sc->sc_openings;
! sc->sc_openings = mpi;
splx(s);
}
Index: i2o/iopspvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/i2o/iopspvar.h,v
retrieving revision 1.5
diff -c -r1.5 iopspvar.h
*** i2o/iopspvar.h 2001/08/04 16:54:18 1.5
--- i2o/iopspvar.h 2003/06/13 02:14:10
***************
*** 64,70 ****
struct iop_initiator sc_ii; /* I2O initiator state */
u_short *sc_tidmap; /* Target/LUN -> TID map */
u_int sc_chgind; /* Last LCT change # */
! u_int sc_curqd; /* Current queue depth */
#ifdef I2OVERBOSE
struct iopsp_target *sc_targetmap; /* Target information */
#endif
--- 64,70 ----
struct iop_initiator sc_ii; /* I2O initiator state */
u_short *sc_tidmap; /* Target/LUN -> TID map */
u_int sc_chgind; /* Last LCT change # */
! int sc_openings; /* # command openings */
#ifdef I2OVERBOSE
struct iopsp_target *sc_targetmap; /* Target information */
#endif
--Apple-Mail-3--145014401--