[BUG] AIC7*** SMP deadlock in ahc_linux_free_device

[BUG] AIC7*** SMP deadlock in ahc_linux_free_device

[Herbert Xu]

Hi:

This bug was discovered by Alex Samad.

The del_timer_sync call in ahc_linux_free_device added in 2.5.67
leads to an SMP deadlock when the function is called from the timer
itself in ahc_linux_dev_timed_unfreeze.

I haven't dug too deeply but it seems that there are also possible
races where ahc_linux_free_device can be called twice on the same
dev.  Once from the timer and then again from a non-timer location.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [BUG] AIC7*** SMP deadlock in ahc_linux_free_device

[Andrew Morton]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> The del_timer_sync call in ahc_linux_free_device added in 2.5.67
> leads to an SMP deadlock when the function is called from the timer
> itself in ahc_linux_dev_timed_unfreeze.

Yup, that will deadlock every time on SMP.  The below should fix it.

> I haven't dug too deeply but it seems that there are also possible
> races where ahc_linux_free_device can be called twice on the same
> dev.  Once from the timer and then again from a non-timer location.

Possibly, although it might be the case that external locking and
refcounting prevent this from happening (hopes).



diff -puN drivers/scsi/aic7xxx/aic7xxx_osm.c~aic7xxx-deadlock-fix drivers/scsi/aic7xxx/aic7xxx_osm.c
--- 25/drivers/scsi/aic7xxx/aic7xxx_osm.c~aic7xxx-deadlock-fix	Tue Feb 17 17:21:44 2004
+++ 25-akpm/drivers/scsi/aic7xxx/aic7xxx_osm.c	Tue Feb 17 17:21:44 2004
@@ -3969,11 +3969,10 @@ ahc_linux_alloc_device(struct ahc_softc 
 }
 
 static void
-ahc_linux_free_device(struct ahc_softc *ahc, struct ahc_linux_device *dev)
+__ahc_linux_free_device(struct ahc_softc *ahc, struct ahc_linux_device *dev)
 {
 	struct ahc_linux_target *targ;
 
-	del_timer_sync(&dev->timer);
 	targ = dev->target;
 	targ->devices[dev->lun] = NULL;
 	free(dev, M_DEVBUF);
@@ -3983,6 +3982,13 @@ ahc_linux_free_device(struct ahc_softc *
 		ahc_linux_free_target(ahc, targ);
 }
 
+static void
+ahc_linux_free_device(struct ahc_softc *ahc, struct ahc_linux_device *dev)
+{
+	del_timer_sync(&dev->timer);
+	__ahc_linux_free_device(ahc, dev);
+}
+
 void
 ahc_send_async(struct ahc_softc *ahc, char channel,
 	       u_int target, u_int lun, ac_code code, void *arg)
@@ -4693,7 +4699,7 @@ ahc_linux_dev_timed_unfreeze(u_long arg)
 		ahc_linux_run_device_queue(ahc, dev);
 	if (TAILQ_EMPTY(&dev->busyq)
 	 && dev->active == 0)
-		ahc_linux_free_device(ahc, dev);
+		__ahc_linux_free_device(ahc, dev);
 	ahc_unlock(ahc, &s);
 }
 

_