hptmv not compatible with WITNESS / INVARIANTS pointers required

[John Baldwin]

On Wednesday 03 January 2007 18:58, Steven Hartland wrote:
> I'm currently trying to debug an issue with Tyan s2892
> based machine but when I enable WITNESS / INVARIANTS
> the Highpoint 182x driver panics the kernel with:
> panic: blocakble sleep lock ( sleep mutex )
> 128 @ vm/uma_core.c: 1845
> 
> I believe this is due to the fact that you cant hold
> a spin mutex while using: bus_dma_tag_create, malloc
> bus_dmamem_alloc etc which the driver currently does,
> but I have no experience with drivers hence dont know
> what it should be doing instead.
> 
> If someone's willing to give me some pointers I'd be
> willing to give creating a patch and testing it a go.
> 
> Here's a snipet of what I think is causing the issue:
> /* This uses a mtx_lock_spin */
>     intrmask_t oldspl = lock_driver();
> 
>     pAdapter->next = 0;
> 
>     if(gIal_Adapter == 0) {
>         gIal_Adapter = pAdapter;
>         pCurAdapter = gIal_Adapter;
>     } else {
>         pCurAdapter->next = pAdapter;
>         pCurAdapter = pAdapter;
>     }
> 
>     pAdapter->outstandingCommands = 0;
> 
>     pMvSataAdapter = &(pAdapter->mvSataAdapter);
>     _vbus_p->OsExt = (void *)pAdapter;
>     pMvSataAdapter->IALData = pAdapter;
> 
> /* Errors due to lock_driver holding mtx_lock_spin?? */
>     if (bus_dma_tag_create(NULL, 1, 0, BUS_SPACE_MAXADDR_32BIT,
>         BUS_SPACE_MAXADDR, NULL, NULL, BUS_SPACE_MAXSIZE_32BIT,
>         MV_MAX_SEGMENTS, BUS_SPACE_MAXSIZE_32BIT, 0, NULL, NULL,
>         &pAdapter->parent_dmat) != 0) {
>         MV_ERROR("RR182x: Failed to create busdma resources\n");
>         unlock_driver(oldspl);
>         return (ENOMEM);
>     }

I would start off making it use a regular mutex (it doesn't need a spin mutex
anyway).  It also doesn't need any locking during the init_adapter() routine 
so I've removed that as well.  Try this:

Index: entry.c
===================================================================
RCS file: /usr/cvs/src/sys/dev/hptmv/entry.c,v
retrieving revision 1.12
diff -u -r1.12 entry.c
--- entry.c	16 May 2006 14:36:25 -0000	1.12
+++ entry.c	4 Jan 2007 17:44:12 -0000
@@ -166,12 +166,12 @@
 {
 
 	intrmask_t spl = 0;
-	mtx_lock_spin(&driver_lock);
+	mtx_lock(&driver_lock);
 	return spl;
 }
 void unlock_driver(intrmask_t spl)
 {
-	mtx_unlock_spin(&driver_lock);
+	mtx_unlock(&driver_lock);
 }
 #else 
 static int driver_locked = 0;
@@ -1168,7 +1168,7 @@
 #if __FreeBSD_version >= 500000
 static void hpt_init(void *dummy)
 {
-	mtx_init(&driver_lock, "hptlock", NULL, MTX_SPIN);
+	mtx_init(&driver_lock, "hptlock", NULL, MTX_DEF);
 }
 SYSINIT(hptinit, SI_SUB_CONFIGURE, SI_ORDER_FIRST, hpt_init, NULL);
 #endif
@@ -1183,8 +1183,6 @@
 
 	PVDevice pVDev;
 
-	intrmask_t oldspl = lock_driver();
-
 	pAdapter->next = 0;
 
 	if(gIal_Adapter == 0){
@@ -1225,7 +1223,6 @@
 	if (hptmv_allocate_edma_queues(pAdapter))
 	{
 		MV_ERROR("RR182x: Failed to allocate memory for EDMA queues\n");
-		unlock_driver(oldspl);
 		return ENOMEM;
 	}
 
@@ -1238,7 +1235,6 @@
 	{
 		MV_ERROR("RR182x: Failed to remap memory space\n");
 		hptmv_free_edma_queues(pAdapter);
-		unlock_driver(oldspl);
 		return ENXIO;
 	}
 	else
@@ -1268,7 +1264,6 @@
 unregister:
 		bus_release_resource(pAdapter->hpt_dev, SYS_RES_MEMORY, rid, 
pAdapter->mem_res);
 		hptmv_free_edma_queues(pAdapter);
-		unlock_driver(oldspl);
 		return ENXIO;
 	}
 	pAdapter->ver_601 = pMvSataAdapter->pcbVersion;
@@ -1411,7 +1406,6 @@
 #endif
 
 	mvSataUnmaskAdapterInterrupt(pMvSataAdapter);
-	unlock_driver(oldspl);
 	return 0;
 }
 

-- 
John Baldwin