PERFORCE change 108878 for review

[John Baldwin]

On Tuesday 31 October 2006 20:12, Scott Long wrote:
> http://perforce.freebsd.org/chv.cgi?CH=108878
> 
> Change 108878 by scottl at scottl-x64 on 2006/11/01 01:11:30
> 
> 	For some wonderful reason, you cannot pass &Giant to msleep.  Work
> 	around that in a crude fashion.  Also add some more assertions.

Ah, yes, that would be most unhappy.  I guess mostly the idea is that Giant 
should be rather implicit and explicitly using Giant for an object lock is 
discouraged.  I'm not sure that is what you are doing though.  I think maybe 
you are borrowing Giant that's already held?

> Affected files ...
> 
> .. //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#14 edit
> 
> Differences ...
> 
> ==== //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#14 (text+ko) 
====
> 
> @@ -515,14 +515,16 @@
>  	/*
>  	 * Interlock the SIM lock with the XPT lock to protect against the
>  	 * SIM going away unexpectedly while we are trying reference it.
> +	xpt_lock_buses();
>  	 */
> -	xpt_lock_buses();
>  
>  	/*
>  	 * Now it's safe to take the SIM lock.
>  	 */
>  	mtx_lock(periph->sim->mtx);
> +	/*
>  	xpt_unlock_buses();
> +	 */
>  
>  	/*
>  	 * Increment the reference count on the peripheral.
> @@ -757,9 +759,11 @@
>  cam_periph_getccb(struct cam_periph *periph, u_int32_t priority)
>  {
>  	struct ccb_hdr *ccb_h;
> +	struct mtx *mtx;
>  	int s;
>  
>  	CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("entering cdgetccb\n"));
> +	mtx_assert(periph->sim->mtx, MA_OWNED);
>  
>  	s = splsoftcam();
>  	
> @@ -770,7 +774,12 @@
>  		if ((SLIST_FIRST(&periph->ccb_list) != NULL)
>  		 && (SLIST_FIRST(&periph->ccb_list)->pinfo.priority == priority))
>  			break;
> -		msleep(&periph->ccb_list, periph->sim->mtx, PRIBIO, "cgticb", 0);
> +		mtx_assert(periph->sim->mtx, MA_OWNED);
> +		if (periph->sim->mtx == &Giant)
> +			mtx = NULL;
> +		else
> +			mtx = periph->sim->mtx;
> +		msleep(&periph->ccb_list, mtx, PRIBIO, "cgticb", 0);
>  	}
>  
>  	ccb_h = SLIST_FIRST(&periph->ccb_list);
> @@ -782,14 +791,19 @@
>  void
>  cam_periph_ccbwait(union ccb *ccb)
>  {
> +	struct mtx *mtx;
>  	struct cam_sim *sim;
>  	int s;
>  
>  	s = splsoftcam();
>  	sim = xpt_path_sim(ccb->ccb_h.path);
> +	if (sim->mtx == &Giant)
> +		mtx = NULL;
> +	else
> +		mtx = sim->mtx;
>  	if ((ccb->ccb_h.pinfo.index != CAM_UNQUEUED_INDEX)
>  	 || ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_INPROG))
> -		msleep(&ccb->ccb_h.cbfcnp, sim->mtx, PRIBIO, "cbwait", 0);
> +		msleep(&ccb->ccb_h.cbfcnp, mtx, PRIBIO, "cbwait", 0);
>  
>  	splx(s);
>  }
> @@ -864,10 +878,13 @@
>  		  cam_flags camflags, u_int32_t sense_flags,
>  		  struct devstat *ds)
>  {
> +	struct cam_sim *sim;
>  	int error;
>   
>  	error = 0;
> -        
> +	sim = xpt_path_sim(ccb->ccb_h.path);
> +
> +	mtx_assert(sim->mtx, MA_OWNED);
>  	/*
>  	 * If the user has supplied a stats structure, and if we understand
>  	 * this particular type of ccb, record the transaction start.
> 

-- 
John Baldwin