From 493e5a3de057bb7de3f89a5405967b9fcc99e399 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Thu, 27 Nov 2008 10:00:02 +1100 Subject: ide: replace the global ide_lock spinlock by per-hwgroup spinlocks (v2) Now that (almost) all host drivers have been fixed not to abuse ide_lock and core code usage of ide_lock has been sanitized we may safely replace ide_lock by per-hwgroup locks. This patch is partially based on earlier patch from Ravikiran G Thirumalai. While at it: - don't use deprecated HWIF() and HWGROUP() macros - update locking documentation in ide.h v2: Add missing spin_lock_init(&hwgroup->lock). (Noticed by Elias Oltmanns) Cc: Vaibhav V. Nivargi Cc: Alok N. Kataria Cc: Shai Fultheim Signed-off-by: Ravikiran Thirumalai Cc: Elias Oltmanns Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-io.c | 33 ++++++++++++++++----------------- drivers/ide/ide-iops.c | 48 +++++++++++++++++++++++------------------------- drivers/ide/ide-park.c | 16 ++++++++-------- drivers/ide/ide-probe.c | 26 +++++++++++++++----------- drivers/ide/ide.c | 8 +++----- drivers/ide/umc8672.c | 11 +++++++---- 6 files changed, 72 insertions(+), 70 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c index 23dfb9b6833b..e1ee4f52c6d2 100644 --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -906,7 +906,7 @@ repeat: /* * Issue a new request to a drive from hwgroup - * Caller must have already done spin_lock_irqsave(&ide_lock, ..); + * Caller must have already done spin_lock_irqsave(&hwgroup->lock, ..); * * A hwgroup is a serialized group of IDE interfaces. Usually there is * exactly one hwif (interface) per hwgroup, but buggy controllers (eg. CMD640) @@ -918,7 +918,7 @@ repeat: * possibly along with many other devices. This is especially common in * PCI-based systems with off-board IDE controller cards. * - * The IDE driver uses the single global ide_lock spinlock to protect + * The IDE driver uses a per-hwgroup spinlock to protect * access to the request queues, and to protect the hwgroup->busy flag. * * The first thread into the driver for a particular hwgroup sets the @@ -934,7 +934,7 @@ repeat: * will start the next request from the queue. If no more work remains, * the driver will clear the hwgroup->busy flag and exit. * - * The ide_lock (spinlock) is used to protect all access to the + * The per-hwgroup spinlock is used to protect all access to the * hwgroup->busy flag, but is otherwise not needed for most processing in * the driver. This makes the driver much more friendlier to shared IRQs * than previous designs, while remaining 100% (?) SMP safe and capable. @@ -947,7 +947,7 @@ static void ide_do_request (ide_hwgroup_t *hwgroup, int masked_irq) ide_startstop_t startstop; int loops = 0; - /* caller must own ide_lock */ + /* caller must own hwgroup->lock */ BUG_ON(!irqs_disabled()); while (!hwgroup->busy) { @@ -1069,11 +1069,11 @@ static void ide_do_request (ide_hwgroup_t *hwgroup, int masked_irq) */ if (masked_irq != IDE_NO_IRQ && hwif->irq != masked_irq) disable_irq_nosync(hwif->irq); - spin_unlock(&ide_lock); + spin_unlock(&hwgroup->lock); local_irq_enable_in_hardirq(); /* allow other IRQs while we start this request */ startstop = start_request(drive, rq); - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); if (masked_irq != IDE_NO_IRQ && hwif->irq != masked_irq) enable_irq(hwif->irq); if (startstop == ide_stopped) @@ -1171,7 +1171,7 @@ void ide_timer_expiry (unsigned long data) unsigned long flags; unsigned long wait = -1; - spin_lock_irqsave(&ide_lock, flags); + spin_lock_irqsave(&hwgroup->lock, flags); if (((handler = hwgroup->handler) == NULL) || (hwgroup->req_gen != hwgroup->req_gen_timer)) { @@ -1204,7 +1204,7 @@ void ide_timer_expiry (unsigned long data) hwgroup->timer.expires = jiffies + wait; hwgroup->req_gen_timer = hwgroup->req_gen; add_timer(&hwgroup->timer); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); return; } } @@ -1214,7 +1214,7 @@ void ide_timer_expiry (unsigned long data) * the handler() function, which means we need to * globally mask the specific IRQ: */ - spin_unlock(&ide_lock); + spin_unlock(&hwgroup->lock); hwif = HWIF(drive); /* disable_irq_nosync ?? */ disable_irq(hwif->irq); @@ -1238,14 +1238,14 @@ void ide_timer_expiry (unsigned long data) hwif->tp_ops->read_status(hwif)); } drive->service_time = jiffies - drive->service_start; - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); enable_irq(hwif->irq); if (startstop == ide_stopped) hwgroup->busy = 0; } } ide_do_request(hwgroup, IDE_NO_IRQ); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); } /** @@ -1338,14 +1338,13 @@ irqreturn_t ide_intr (int irq, void *dev_id) { unsigned long flags; ide_hwgroup_t *hwgroup = (ide_hwgroup_t *)dev_id; - ide_hwif_t *hwif; + ide_hwif_t *hwif = hwgroup->hwif; ide_drive_t *drive; ide_handler_t *handler; ide_startstop_t startstop; irqreturn_t irq_ret = IRQ_NONE; - spin_lock_irqsave(&ide_lock, flags); - hwif = hwgroup->hwif; + spin_lock_irqsave(&hwgroup->lock, flags); if (!ide_ack_intr(hwif)) goto out; @@ -1415,7 +1414,7 @@ irqreturn_t ide_intr (int irq, void *dev_id) hwgroup->handler = NULL; hwgroup->req_gen++; del_timer(&hwgroup->timer); - spin_unlock(&ide_lock); + spin_unlock(&hwgroup->lock); if (hwif->port_ops && hwif->port_ops->clear_irq) hwif->port_ops->clear_irq(drive); @@ -1426,7 +1425,7 @@ irqreturn_t ide_intr (int irq, void *dev_id) /* service this interrupt, may set handler for next interrupt */ startstop = handler(drive); - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); /* * Note that handler() may have set things up for another * interrupt to occur soon, but it cannot happen until @@ -1447,7 +1446,7 @@ irqreturn_t ide_intr (int irq, void *dev_id) out_handled: irq_ret = IRQ_HANDLED; out: - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); return irq_ret; } diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c index 6f57de85dcd9..883cfbfff035 100644 --- a/drivers/ide/ide-iops.c +++ b/drivers/ide/ide-iops.c @@ -840,10 +840,12 @@ static void __ide_set_handler (ide_drive_t *drive, ide_handler_t *handler, void ide_set_handler (ide_drive_t *drive, ide_handler_t *handler, unsigned int timeout, ide_expiry_t *expiry) { + ide_hwgroup_t *hwgroup = drive->hwif->hwgroup; unsigned long flags; - spin_lock_irqsave(&ide_lock, flags); + + spin_lock_irqsave(&hwgroup->lock, flags); __ide_set_handler(drive, handler, timeout, expiry); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); } EXPORT_SYMBOL(ide_set_handler); @@ -865,10 +867,11 @@ EXPORT_SYMBOL(ide_set_handler); void ide_execute_command(ide_drive_t *drive, u8 cmd, ide_handler_t *handler, unsigned timeout, ide_expiry_t *expiry) { + ide_hwif_t *hwif = drive->hwif; + ide_hwgroup_t *hwgroup = hwif->hwgroup; unsigned long flags; - ide_hwif_t *hwif = HWIF(drive); - spin_lock_irqsave(&ide_lock, flags); + spin_lock_irqsave(&hwgroup->lock, flags); __ide_set_handler(drive, handler, timeout, expiry); hwif->tp_ops->exec_command(hwif, cmd); /* @@ -878,19 +881,20 @@ void ide_execute_command(ide_drive_t *drive, u8 cmd, ide_handler_t *handler, * FIXME: we could skip this delay with care on non shared devices */ ndelay(400); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); } EXPORT_SYMBOL(ide_execute_command); void ide_execute_pkt_cmd(ide_drive_t *drive) { ide_hwif_t *hwif = drive->hwif; + ide_hwgroup_t *hwgroup = hwif->hwgroup; unsigned long flags; - spin_lock_irqsave(&ide_lock, flags); + spin_lock_irqsave(&hwgroup->lock, flags); hwif->tp_ops->exec_command(hwif, ATA_CMD_PACKET); ndelay(400); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); } EXPORT_SYMBOL_GPL(ide_execute_pkt_cmd); @@ -1081,22 +1085,16 @@ static void pre_reset(ide_drive_t *drive) */ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi) { - unsigned int unit; - unsigned long flags, timeout; - ide_hwif_t *hwif; - ide_hwgroup_t *hwgroup; - struct ide_io_ports *io_ports; - const struct ide_tp_ops *tp_ops; + ide_hwif_t *hwif = drive->hwif; + ide_hwgroup_t *hwgroup = hwif->hwgroup; + struct ide_io_ports *io_ports = &hwif->io_ports; + const struct ide_tp_ops *tp_ops = hwif->tp_ops; const struct ide_port_ops *port_ops; + unsigned long flags, timeout; + unsigned int unit; DEFINE_WAIT(wait); - spin_lock_irqsave(&ide_lock, flags); - hwif = HWIF(drive); - hwgroup = HWGROUP(drive); - - io_ports = &hwif->io_ports; - - tp_ops = hwif->tp_ops; + spin_lock_irqsave(&hwgroup->lock, flags); /* We must not reset with running handlers */ BUG_ON(hwgroup->handler != NULL); @@ -1111,7 +1109,7 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi) hwgroup->poll_timeout = jiffies + WAIT_WORSTCASE; hwgroup->polling = 1; __ide_set_handler(drive, &atapi_reset_pollfunc, HZ/20, NULL); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); return ide_started; } @@ -1134,9 +1132,9 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi) if (time_before_eq(timeout, now)) break; - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); timeout = schedule_timeout_uninterruptible(timeout - now); - spin_lock_irqsave(&ide_lock, flags); + spin_lock_irqsave(&hwgroup->lock, flags); } while (timeout); finish_wait(&ide_park_wq, &wait); @@ -1148,7 +1146,7 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi) pre_reset(&hwif->drives[unit]); if (io_ports->ctl_addr == 0) { - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); ide_complete_drive_reset(drive, -ENXIO); return ide_stopped; } @@ -1184,7 +1182,7 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi) if (port_ops && port_ops->resetproc) port_ops->resetproc(drive); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); return ide_started; } diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c index 03b00e57e93f..63d01c55f865 100644 --- a/drivers/ide/ide-park.c +++ b/drivers/ide/ide-park.c @@ -7,17 +7,16 @@ DECLARE_WAIT_QUEUE_HEAD(ide_park_wq); static void issue_park_cmd(ide_drive_t *drive, unsigned long timeout) { + ide_hwgroup_t *hwgroup = drive->hwif->hwgroup; struct request_queue *q = drive->queue; struct request *rq; int rc; timeout += jiffies; - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); if (drive->dev_flags & IDE_DFLAG_PARKED) { - ide_hwgroup_t *hwgroup = drive->hwif->hwgroup; - int reset_timer; + int reset_timer = time_before(timeout, drive->sleep); - reset_timer = time_before(timeout, drive->sleep); drive->sleep = timeout; wake_up_all(&ide_park_wq); if (reset_timer && hwgroup->sleeping && @@ -26,10 +25,10 @@ static void issue_park_cmd(ide_drive_t *drive, unsigned long timeout) hwgroup->busy = 0; blk_start_queueing(q); } - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); return; } - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); rq = blk_get_request(q, READ, __GFP_WAIT); rq->cmd[0] = REQ_PARK_HEADS; @@ -62,20 +61,21 @@ ssize_t ide_park_show(struct device *dev, struct device_attribute *attr, char *buf) { ide_drive_t *drive = to_ide_device(dev); + ide_hwgroup_t *hwgroup = drive->hwif->hwgroup; unsigned long now; unsigned int msecs; if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD) return -EOPNOTSUPP; - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); now = jiffies; if (drive->dev_flags & IDE_DFLAG_PARKED && time_after(drive->sleep, now)) msecs = jiffies_to_msecs(drive->sleep - now); else msecs = 0; - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); return snprintf(buf, 20, "%u\n", msecs); } diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c index c55bdbd22314..504bc9480e93 100644 --- a/drivers/ide/ide-probe.c +++ b/drivers/ide/ide-probe.c @@ -906,7 +906,8 @@ static int ide_init_queue(ide_drive_t *drive) * do not. */ - q = blk_init_queue_node(do_ide_request, &ide_lock, hwif_to_node(hwif)); + q = blk_init_queue_node(do_ide_request, &hwif->hwgroup->lock, + hwif_to_node(hwif)); if (!q) return 1; @@ -947,7 +948,7 @@ static void ide_add_drive_to_hwgroup(ide_drive_t *drive) { ide_hwgroup_t *hwgroup = drive->hwif->hwgroup; - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); if (!hwgroup->drive) { /* first drive for hwgroup. */ drive->next = drive; @@ -957,7 +958,7 @@ static void ide_add_drive_to_hwgroup(ide_drive_t *drive) drive->next = hwgroup->drive->next; hwgroup->drive->next = drive; } - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); } /* @@ -1002,7 +1003,7 @@ void ide_remove_port_from_hwgroup(ide_hwif_t *hwif) ide_ports[hwif->index] = NULL; - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); /* * Remove us from the hwgroup, and free * the hwgroup if we were the only member @@ -1030,7 +1031,7 @@ void ide_remove_port_from_hwgroup(ide_hwif_t *hwif) } BUG_ON(hwgroup->hwif == hwif); } - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); } /* @@ -1092,17 +1093,19 @@ static int init_irq (ide_hwif_t *hwif) * linked list, the first entry is the hwif that owns * hwgroup->handler - do not change that. */ - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); hwif->next = hwgroup->hwif->next; hwgroup->hwif->next = hwif; BUG_ON(hwif->next == hwif); - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); } else { hwgroup = kmalloc_node(sizeof(*hwgroup), GFP_KERNEL|__GFP_ZERO, hwif_to_node(hwif)); if (hwgroup == NULL) goto out_up; + spin_lock_init(&hwgroup->lock); + hwif->hwgroup = hwgroup; hwgroup->hwif = hwif->next = hwif; @@ -1263,20 +1266,21 @@ static void ide_remove_drive_from_hwgroup(ide_drive_t *drive) static void drive_release_dev (struct device *dev) { ide_drive_t *drive = container_of(dev, ide_drive_t, gendev); + ide_hwgroup_t *hwgroup = drive->hwif->hwgroup; ide_proc_unregister_device(drive); - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); ide_remove_drive_from_hwgroup(drive); kfree(drive->id); drive->id = NULL; drive->dev_flags &= ~IDE_DFLAG_PRESENT; /* Messed up locking ... */ - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); blk_cleanup_queue(drive->queue); - spin_lock_irq(&ide_lock); + spin_lock_irq(&hwgroup->lock); drive->queue = NULL; - spin_unlock_irq(&ide_lock); + spin_unlock_irq(&hwgroup->lock); complete(&drive->gendev_rel_comp); } diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c index bca6877ee6a5..41d042053548 100644 --- a/drivers/ide/ide.c +++ b/drivers/ide/ide.c @@ -74,9 +74,6 @@ static const u8 ide_hwif_to_major[] = { IDE0_MAJOR, IDE1_MAJOR, DEFINE_MUTEX(ide_cfg_mtx); -__cacheline_aligned_in_smp DEFINE_SPINLOCK(ide_lock); -EXPORT_SYMBOL(ide_lock); - static void ide_port_init_devices_data(ide_hwif_t *); /* @@ -333,6 +330,7 @@ static int set_pio_mode_abuse(ide_hwif_t *hwif, u8 req_pio) static int set_pio_mode(ide_drive_t *drive, int arg) { ide_hwif_t *hwif = drive->hwif; + ide_hwgroup_t *hwgroup = hwif->hwgroup; const struct ide_port_ops *port_ops = hwif->port_ops; if (arg < 0 || arg > 255) @@ -347,9 +345,9 @@ static int set_pio_mode(ide_drive_t *drive, int arg) unsigned long flags; /* take lock for IDE_DFLAG_[NO_]UNMASK/[NO_]IO_32BIT */ - spin_lock_irqsave(&ide_lock, flags); + spin_lock_irqsave(&hwgroup->lock, flags); port_ops->set_pio_mode(drive, arg); - spin_unlock_irqrestore(&ide_lock, flags); + spin_unlock_irqrestore(&hwgroup->lock, flags); } else port_ops->set_pio_mode(drive, arg); } else { diff --git a/drivers/ide/umc8672.c b/drivers/ide/umc8672.c index 1da076e0c917..e29978cf6197 100644 --- a/drivers/ide/umc8672.c +++ b/drivers/ide/umc8672.c @@ -107,18 +107,21 @@ static void umc_set_speeds(u8 speeds[]) static void umc_set_pio_mode(ide_drive_t *drive, const u8 pio) { ide_hwif_t *hwif = drive->hwif; - unsigned long flags; + ide_hwgroup_t *mate_hwgroup = hwif->mate ? hwif->mate->hwgroup : NULL; + unsigned long uninitialized_var(flags); printk("%s: setting umc8672 to PIO mode%d (speed %d)\n", drive->name, pio, pio_to_umc[pio]); - spin_lock_irqsave(&ide_lock, flags); - if (hwif->mate && hwif->mate->hwgroup->handler) { + if (mate_hwgroup) + spin_lock_irqsave(&mate_hwgroup->lock, flags); + if (mate_hwgroup && mate_hwgroup->handler) { printk(KERN_ERR "umc8672: other interface is busy: exiting tune_umc()\n"); } else { current_speeds[drive->name[2] - 'a'] = pio_to_umc[pio]; umc_set_speeds(current_speeds); } - spin_unlock_irqrestore(&ide_lock, flags); + if (mate_hwgroup) + spin_unlock_irqrestore(&mate_hwgroup->lock, flags); } static const struct ide_port_ops umc8672_port_ops = { -- cgit v1.2.3