Date: Thu, 4 Jan 2007 12:45:34 -0500 From: John Baldwin <jhb@freebsd.org> To: freebsd-hackers@freebsd.org Cc: Steven Hartland <killing@multiplay.co.uk> Subject: Re: hptmv not compatible with WITNESS / INVARIANTS pointers required Message-ID: <200701041245.34582.jhb@freebsd.org> In-Reply-To: <006301c72f93$0ffceb40$b3db87d4@multiplay.co.uk> References: <006301c72f93$0ffceb40$b3db87d4@multiplay.co.uk>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200701041245.34582.jhb>