Re: Deadlock between GEOM and devfs device destroy and process exit.

Pawel Jakub Dawidek wrote:
> On Sat, Jan 30, 2010 at 12:44:51PM +0100, Pawel Jakub Dawidek wrote:
>> Maybe I'll add how I understand what's going on:
>>
>> GEOM calls destroy_dev() while holding the topology lock.
>>
>> Destroy_dev() wants to destroy device, but can't because there are
>> threads that still have it open.
>>
>> The threads can't close it, because to close it they need the topology
>> lock.
>>
>> The deadlock is quite obvious, IMHO.
> 
> Guys, changing destroy_dev() to destroy_dev_sched() in geom_dev.c fixes
> the problem for me (at least it makes race window so small that I can't
> reproduce it). Is there anyone who isn't happy with such a change?

Have you done some locking there? Because my system crashes with such
straightforward change, when g_dev_close() called after geom node being
destroyed. Attached patch fixes that for me, but I have doubts that it
is complete. There is still seems to be a race with new I/O requests and
ioctl's, that are not protected by topology lock. At least if devfs code
doesn't handle it somehow.

-- 
Alexander Motin

--- geom_dev.c.prev	2010-01-30 05:22:36.000000000 +0200
+++ geom_dev.c	2010-02-01 11:55:53.000000000 +0200
_at__at_ -184,10 +184,13 _at__at_ g_dev_close(struct cdev *dev, int flags,
 	struct g_consumer *cp;
 	int error, r, w, e, i;
 
+	g_topology_lock();
 	gp = dev->si_drv1;
 	cp = dev->si_drv2;
-	if (gp == NULL || cp == NULL)
+	if (gp == NULL || cp == NULL) {
+		g_topology_unlock();
 		return(ENXIO);
+	}
 	g_trace(G_T_ACCESS, "g_dev_close(%s, %d, %d, %p)",
 	    gp->name, flags, fmt, td);
 	r = flags & FREAD ? -1 : 0;
_at__at_ -197,7 +200,6 _at__at_ g_dev_close(struct cdev *dev, int flags,
 #else
 	e = 0;
 #endif
-	g_topology_lock();
 	if (dev->si_devsw == NULL)
 		error = ENXIO;		/* We were orphaned */
 	else
_at__at_ -435,7 +437,10 _at__at_ g_dev_orphan(struct g_consumer *cp)
 		set_dumper(NULL);
 
 	/* Destroy the struct cdev *so we get no more requests */
-	destroy_dev(dev);
+	gp->softc = NULL;
+	dev->si_drv1 = NULL;
+	dev->si_drv2 = NULL;
+	destroy_dev_sched(dev);
 
 	/* Wait for the cows to come home */
 	while (cp->nstart != cp->nend)