From: James Bottomley <James.Bottomley@steeleye.com>
To: gibbs@scsiguy.com
Cc: SCSI Mailing List <linux-scsi@vger.kernel.org>,
Andrew Morton <akpm@osdl.org>
Subject: [PATCH] Fix aic7xxx del_timer_sync() deadlock
Date: 27 Feb 2004 12:26:22 -0600 [thread overview]
Message-ID: <1077906383.2157.98.camel@mulgrave> (raw)
The deadlock is quite simple: if the device timer ever fires, it can
call a linux_free_device() from the timer which will do a del_timer_sync
on the timer and hang.
The best fix, I think is to eliminate this timer from the driver. All
it was doing was providing a time limited queue freeze for the BUSY and
QUEUE_FULL on no outstanding commands cases. These are handled
correctly by the mid-layer, so there's no need for the driver do try to
handle them on it's own.
It looks like the only other consumer of the queue freezing api is an
untimed tag/untagged switch, so I pulled out the per device timer as
well.
James
===== drivers/scsi/aic7xxx/aic79xx_osm.c 1.52 vs edited =====
--- 1.52/drivers/scsi/aic7xxx/aic79xx_osm.c Wed Feb 18 21:42:35 2004
+++ edited/drivers/scsi/aic7xxx/aic79xx_osm.c Fri Feb 27 12:20:52 2004
@@ -472,7 +472,6 @@
Scsi_Cmnd *cmd);
static void ahd_linux_filter_inquiry(struct ahd_softc *ahd,
struct ahd_devinfo *devinfo);
-static void ahd_linux_dev_timed_unfreeze(u_long arg);
static void ahd_linux_sem_timeout(u_long arg);
static void ahd_linux_initialize_scsi_bus(struct ahd_softc *ahd);
static void ahd_linux_size_nseg(void);
@@ -4262,7 +4261,6 @@
if (dev == NULL)
return (NULL);
memset(dev, 0, sizeof(*dev));
- init_timer(&dev->timer);
TAILQ_INIT(&dev->busyq);
dev->flags = AHD_DEV_UNCONFIGURED;
dev->lun = lun;
@@ -4291,7 +4289,6 @@
{
struct ahd_linux_target *targ;
- del_timer(&dev->timer);
targ = dev->target;
targ->devices[dev->lun] = NULL;
free(dev, M_DEVBUF);
@@ -4683,28 +4680,9 @@
(dev->flags & AHD_DEV_Q_BASIC)
? AHD_QUEUE_BASIC : AHD_QUEUE_TAGGED);
ahd_set_scsi_status(scb, SCSI_STATUS_BUSY);
- /* FALLTHROUGH */
- }
- case SCSI_STATUS_BUSY:
- /*
- * Set a short timer to defer sending commands for
- * a bit since Linux will not delay in this case.
- */
- if ((dev->flags & AHD_DEV_TIMER_ACTIVE) != 0) {
- printf("%s:%c:%d: Device Timer still active during "
- "busy processing\n", ahd_name(ahd),
- dev->target->channel, dev->target->target);
- break;
- }
- dev->flags |= AHD_DEV_TIMER_ACTIVE;
- dev->qfrozen++;
- init_timer(&dev->timer);
- dev->timer.data = (u_long)dev;
- dev->timer.expires = jiffies + (HZ/2);
- dev->timer.function = ahd_linux_dev_timed_unfreeze;
- add_timer(&dev->timer);
break;
}
+ }
}
static void
@@ -5017,28 +4995,6 @@
scb->platform_data->flags &= ~AHD_SCB_UP_EH_SEM;
up(&ahd->platform_data->eh_sem);
}
- ahd_unlock(ahd, &s);
-}
-
-static void
-ahd_linux_dev_timed_unfreeze(u_long arg)
-{
- struct ahd_linux_device *dev;
- struct ahd_softc *ahd;
- u_long s;
-
- dev = (struct ahd_linux_device *)arg;
- ahd = dev->target->ahd;
- ahd_lock(ahd, &s);
- dev->flags &= ~AHD_DEV_TIMER_ACTIVE;
- if (dev->qfrozen > 0)
- dev->qfrozen--;
- if (dev->qfrozen == 0
- && (dev->flags & AHD_DEV_ON_RUN_LIST) == 0)
- ahd_linux_run_device_queue(ahd, dev);
- if ((dev->flags & AHD_DEV_UNCONFIGURED) != 0
- && dev->active == 0)
- ahd_linux_free_device(ahd, dev);
ahd_unlock(ahd, &s);
}
===== drivers/scsi/aic7xxx/aic79xx_osm.h 1.35 vs edited =====
--- 1.35/drivers/scsi/aic7xxx/aic79xx_osm.h Tue Dec 30 12:12:35 2003
+++ edited/drivers/scsi/aic7xxx/aic79xx_osm.h Fri Feb 27 12:16:56 2004
@@ -387,11 +387,6 @@
ahd_linux_dev_flags flags;
/*
- * Per device timer.
- */
- struct timer_list timer;
-
- /*
* The high limit for the tags variable.
*/
u_int maxtags;
===== drivers/scsi/aic7xxx/aic7xxx_osm.c 1.49 vs edited =====
--- 1.49/drivers/scsi/aic7xxx/aic7xxx_osm.c Wed Feb 18 21:42:35 2004
+++ edited/drivers/scsi/aic7xxx/aic7xxx_osm.c Fri Feb 27 12:12:51 2004
@@ -490,7 +490,6 @@
static void ahc_linux_sem_timeout(u_long arg);
static void ahc_linux_freeze_simq(struct ahc_softc *ahc);
static void ahc_linux_release_simq(u_long arg);
-static void ahc_linux_dev_timed_unfreeze(u_long arg);
static int ahc_linux_queue_recovery_cmd(Scsi_Cmnd *cmd, scb_flag flag);
static void ahc_linux_initialize_scsi_bus(struct ahc_softc *ahc);
static void ahc_linux_size_nseg(void);
@@ -3944,7 +3943,6 @@
if (dev == NULL)
return (NULL);
memset(dev, 0, sizeof(*dev));
- init_timer(&dev->timer);
TAILQ_INIT(&dev->busyq);
dev->flags = AHC_DEV_UNCONFIGURED;
dev->lun = lun;
@@ -3973,7 +3971,6 @@
{
struct ahc_linux_target *targ;
- del_timer_sync(&dev->timer);
targ = dev->target;
targ->devices[dev->lun] = NULL;
free(dev, M_DEVBUF);
@@ -4363,27 +4360,6 @@
ahc_platform_set_tags(ahc, &devinfo,
(dev->flags & AHC_DEV_Q_BASIC)
? AHC_QUEUE_BASIC : AHC_QUEUE_TAGGED);
- /* FALLTHROUGH */
- }
- case SCSI_STATUS_BUSY:
- {
- /*
- * Set a short timer to defer sending commands for
- * a bit since Linux will not delay in this case.
- */
- if ((dev->flags & AHC_DEV_TIMER_ACTIVE) != 0) {
- printf("%s:%c:%d: Device Timer still active during "
- "busy processing\n", ahc_name(ahc),
- dev->target->channel, dev->target->target);
- break;
- }
- dev->flags |= AHC_DEV_TIMER_ACTIVE;
- dev->qfrozen++;
- init_timer(&dev->timer);
- dev->timer.data = (u_long)dev;
- dev->timer.expires = jiffies + (HZ/2);
- dev->timer.function = ahc_linux_dev_timed_unfreeze;
- add_timer(&dev->timer);
break;
}
}
@@ -4673,28 +4649,6 @@
*/
if (unblock_reqs)
scsi_unblock_requests(ahc->platform_data->host);
-}
-
-static void
-ahc_linux_dev_timed_unfreeze(u_long arg)
-{
- struct ahc_linux_device *dev;
- struct ahc_softc *ahc;
- u_long s;
-
- dev = (struct ahc_linux_device *)arg;
- ahc = dev->target->ahc;
- ahc_lock(ahc, &s);
- dev->flags &= ~AHC_DEV_TIMER_ACTIVE;
- if (dev->qfrozen > 0)
- dev->qfrozen--;
- if (dev->qfrozen == 0
- && (dev->flags & AHC_DEV_ON_RUN_LIST) == 0)
- ahc_linux_run_device_queue(ahc, dev);
- if (TAILQ_EMPTY(&dev->busyq)
- && dev->active == 0)
- ahc_linux_free_device(ahc, dev);
- ahc_unlock(ahc, &s);
}
static int
===== drivers/scsi/aic7xxx/aic7xxx_osm.h 1.51 vs edited =====
--- 1.51/drivers/scsi/aic7xxx/aic7xxx_osm.h Tue Dec 30 12:12:37 2003
+++ edited/drivers/scsi/aic7xxx/aic7xxx_osm.h Fri Feb 27 12:13:34 2004
@@ -399,11 +399,6 @@
ahc_linux_dev_flags flags;
/*
- * Per device timer.
- */
- struct timer_list timer;
-
- /*
* The high limit for the tags variable.
*/
u_int maxtags;
next reply other threads:[~2004-02-27 18:26 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-02-27 18:26 James Bottomley [this message]
2004-02-27 19:23 ` [PATCH] Fix aic7xxx del_timer_sync() deadlock Justin T. Gibbs
2004-02-27 19:34 ` James Bottomley
2004-02-27 20:50 ` Justin T. Gibbs
2004-02-28 15:39 ` James Bottomley
2004-02-29 19:26 ` Justin T. Gibbs
2004-02-29 21:10 ` James Bottomley
2004-02-29 22:23 ` Justin T. Gibbs
2004-02-29 21:25 ` James Bottomley
2004-02-28 2:40 ` Jeff Garzik
2004-02-28 9:25 ` Jens Axboe
2004-02-28 23:50 ` Jeff Garzik
2004-02-29 9:13 ` Jens Axboe
2004-02-29 16:21 ` Jeff Garzik
2004-02-29 16:39 ` Jens Axboe
2004-02-29 17:28 ` Jeff Garzik
2004-02-29 17:55 ` Jens Axboe
2004-02-29 18:57 ` Justin T. Gibbs
2004-02-29 19:00 ` Jeff Garzik
2004-02-29 19:28 ` Justin T. Gibbs
2004-02-29 19:37 ` Jeff Garzik
2004-02-29 19:42 ` Justin T. Gibbs
2004-02-29 19:44 ` Jeff Garzik
2004-02-29 20:06 ` Jens Axboe
2004-02-29 20:20 ` Jeff Garzik
2004-02-29 20:27 ` Jens Axboe
2004-02-29 20:28 ` Jeff Garzik
2004-02-29 19:43 ` Jeff Garzik
2004-02-29 20:04 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1077906383.2157.98.camel@mulgrave \
--to=james.bottomley@steeleye.com \
--cc=akpm@osdl.org \
--cc=gibbs@scsiguy.com \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.