[PATCH 5/6] ide: use lock bitops for ports serialization

Bartlomiej Zolnierkiewicz
2008-12-13T23:42:08Z

Subject: [PATCH] ide: use lock bitops for ports serialization

* Add ->host_busy field to struct ide_host and use it's first bit
together with lock bitops to provide new ports serialization method.

* Convert core IDE code to use new ide_[un]lock_host() helpers.

This removes the need for taking hwgroup->lock if host is already
busy on serialized hosts and makes it possible to merge ide_hwgroup_t
into ide_hwif_t (done in the later patch).

* Remove no longer needed ide_hwgroup_t.busy and ide_[un]lock_hwgroup().

* Update do_ide_request() documentation.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-io.c | 77 ++++++++++++++++++++++++---------------------------
include/linux/ide.h | 34 +++-------------------
2 files changed, 42 insertions(+), 69 deletions(-)

Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -666,43 +666,36 @@ void ide_stall_queue (ide_drive_t *drive
}
EXPORT_SYMBOL(ide_stall_queue);

+static inline int ide_lock_host(struct ide_host *host, ide_hwif_t *hwif)
+{
+ int rc = 0;
+
+ if (host->host_flags & IDE_HFLAG_SERIALIZE) {
+ rc = test_and_set_bit_lock(IDE_HOST_BUSY, &host->host_busy);
+ if (rc == 0) {
+ /* for atari only */
+ ide_get_lock(ide_intr, hwif);
+ }
+ }
+ return rc;
+}
+
+static inline void ide_unlock_host(struct ide_host *host)
+{
+ /* for atari only */
+ ide_release_lock();
+ if (host->host_flags & IDE_HFLAG_SERIALIZE)
+ clear_bit_unlock(IDE_HOST_BUSY, &host->host_busy);
+}
+
/*
* Issue a new request to a drive from hwgroup
- *
- * A hwgroup is a serialized group of IDE interfaces. Usually there is
- * exactly one hwif (interface) per hwgroup, but buggy controllers (eg. CMD640)
- * may have both interfaces in a single hwgroup to "serialize" access.
- * Or possibly multiple ISA interfaces can share a common IRQ by being grouped
- * together into one hwgroup for serialized access.
- *
- * Note also that several hwgroups can end up sharing a single IRQ,
- * possibly along with many other devices. This is especially common in
- * PCI-based systems with off-board IDE controller cards.
- *
- * The IDE driver uses a per-hwgroup lock to protect the hwgroup->busy flag.
- *
- * The first thread into the driver for a particular hwgroup sets the
- * hwgroup->busy flag to indicate that this hwgroup is now active,
- * and then initiates processing of the top request from the request queue.
- *
- * Other threads attempting entry notice the busy setting, and will simply
- * queue their new requests and exit immediately. Note that hwgroup->busy
- * remains set even when the driver is merely awaiting the next interrupt.
- * Thus, the meaning is "this hwgroup is busy processing a request".
- *
- * When processing of a request completes, the completing thread or IRQ-handler
- * will start the next request from the queue. If no more work remains,
- * the driver will clear the hwgroup->busy flag and exit.
- *
- * 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.
*/
void do_ide_request(struct request_queue *q)
{
ide_drive_t *drive = q->queuedata;
ide_hwif_t *hwif = drive->hwif;
+ struct ide_host *host = hwif->host;
ide_hwgroup_t *hwgroup = hwif->hwgroup;
struct request *rq;
ide_startstop_t startstop;
@@ -721,9 +714,13 @@ void do_ide_request(struct request_queue
blk_remove_plug(q);

spin_unlock_irq(q->queue_lock);
+
+ if (ide_lock_host(host, hwif))
+ goto plug_device_2;
+
spin_lock_irq(&hwgroup->lock);

- if (!ide_lock_hwgroup(hwgroup, hwif)) {
+ if (1) {
ide_hwif_t *prev_port;
repeat:
prev_port = hwif->host->cur_port;
@@ -731,7 +728,7 @@ repeat:

if (drive->dev_flags & IDE_DFLAG_SLEEPING) {
if (time_before(drive->sleep, jiffies)) {
- ide_unlock_hwgroup(hwgroup);
+ ide_unlock_host(host);
goto plug_device;
}
}
@@ -761,7 +758,7 @@ repeat:
spin_lock_irq(&hwgroup->lock);

if (!rq) {
- ide_unlock_hwgroup(hwgroup);
+ ide_unlock_host(host);
goto out;
}

@@ -782,7 +779,7 @@ repeat:
blk_pm_request(rq) == 0 &&
(rq->cmd_flags & REQ_PREEMPT) == 0) {
/* there should be no pending command at this point */
- ide_unlock_hwgroup(hwgroup);
+ ide_unlock_host(host);
goto plug_device;
}

@@ -794,8 +791,7 @@ repeat:

if (startstop == ide_stopped)
goto repeat;
- } else
- goto plug_device;
+ }
out:
spin_unlock_irq(&hwgroup->lock);
spin_lock_irq(q->queue_lock);
@@ -803,6 +799,7 @@ out:

plug_device:
spin_unlock_irq(&hwgroup->lock);
+plug_device_2:
spin_lock_irq(q->queue_lock);

if (!elv_queue_empty(q))
@@ -844,9 +841,9 @@ static ide_startstop_t ide_dma_timeout_r
ide_dma_off_quietly(drive);

/*
- * un-busy drive etc (hwgroup->busy is cleared on return) and
- * make sure request is sane
+ * un-busy drive etc and make sure request is sane
*/
+
rq = HWGROUP(drive)->rq;

if (!rq)
@@ -964,7 +961,7 @@ void ide_timer_expiry (unsigned long dat
spin_lock_irq(&hwgroup->lock);
enable_irq(hwif->irq);
if (startstop == ide_stopped) {
- ide_unlock_hwgroup(hwgroup);
+ ide_unlock_host(hwif->host);
plug_device = 1;
}
}
@@ -1150,7 +1147,7 @@ irqreturn_t ide_intr (int irq, void *dev
*/
if (startstop == ide_stopped) {
if (hwgroup->handler == NULL) { /* paranoia */
- ide_unlock_hwgroup(hwgroup);
+ ide_unlock_host(hwif->host);
plug_device = 1;
} else
printk(KERN_ERR "%s: %s: huh? expected NULL handler "
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -851,8 +851,13 @@ struct ide_host {
unsigned long host_flags;
void *host_priv;
ide_hwif_t *cur_port; /* for hosts requiring serialization */
+
+ /* used for hosts requiring serialization */
+ volatile long host_busy;
};

+#define IDE_HOST_BUSY 0
+
/*
* internal ide interrupt handler type
*/
@@ -866,8 +871,6 @@ typedef struct hwgroup_s {
/* irq handler, if active */
ide_startstop_t (*handler)(ide_drive_t *);

- /* BOOL: protects all fields below */
- volatile int busy;
/* BOOL: polling active & poll_timeout field valid */
unsigned int polling : 1;

@@ -1271,26 +1274,6 @@ extern void ide_stall_queue(ide_drive_t

extern void ide_timer_expiry(unsigned long);
extern irqreturn_t ide_intr(int irq, void *dev_id);
-
-static inline int ide_lock_hwgroup(ide_hwgroup_t *hwgroup, ide_hwif_t *hwif)
-{
- if (hwgroup->busy)
- return 1;
-
- hwgroup->busy = 1;
- /* for atari only */
- ide_get_lock(ide_intr, hwif);
-
- return 0;
-}
-
-static inline void ide_unlock_hwgroup(ide_hwgroup_t *hwgroup)
-{
- /* for atari only */
- ide_release_lock();
- hwgroup->busy = 0;
-}
-
extern void do_ide_request(struct request_queue *);

void ide_init_disk(struct gendisk *, ide_drive_t *);
@@ -1617,13 +1600,6 @@ static inline void ide_set_max_pio(ide_d

extern spinlock_t ide_lock;
extern struct mutex ide_cfg_mtx;
-/*
- * Structure locking:
- *
- * ide_hwgroup_t->busy: hwgroup->lock
- * ide_hwif_t->{hwgroup,mate}: constant, no locking
- * ide_drive_t->hwif: constant, no locking
- */

#define local_irq_set(flags) do { local_save_flags((flags)); local_irq_enable_in_hardirq(); } while (0)