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--