From c0c362b60e259e3480a36ef70280d545818844f0 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 6 Sep 2010 17:57:14 +0200 Subject: libata: implement cross-port EH exclusion In libata, the non-EH code paths should always take and release ap->lock explicitly when accessing hardware or shared data structures. However, once EH is active, it's assumed that the port is owned by EH and EH methods don't explicitly take ap->lock unless race from irq handler or other code paths are expected. However, libata EH didn't guarantee exclusion among EHs for ports of the same host. IOW, multiple EHs may execute in parallel on multiple ports of the same controller. In many cases, especially in SATA, the ports are completely independent of each other and this doesn't cause problems; however, there are cases where different ports share the same resource, which lead to obscure timing related bugs such as the one fixed by commit 213373cf (ata_piix: fix locking around SIDPR access). This patch implements exclusion among EHs of the same host. When EH begins, it acquires per-host EH ownership by calling ata_eh_acquire(). When EH finishes, the ownership is released by calling ata_eh_release(). EH ownership is also released whenever the EH thread goes to sleep from ata_msleep() or explicitly and reacquired after waking up. This ensures that while EH is actively accessing the hardware, it has exclusive access to it while allowing EHs to interleave and progress in parallel as they hit waiting stages, which dominate the time spent in EH. This achieves cross-port EH exclusion without pervasive and fragile changes while still allowing parallel EH for the most part. This was first reported by yuanding02@gmail.com more than three years ago in the following bugzilla. :-) https://bugzilla.kernel.org/show_bug.cgi?id=8223 Signed-off-by: Tejun Heo Cc: Alan Cox Reported-by: yuanding02@gmail.com Signed-off-by: Jeff Garzik --- drivers/ata/libata-eh.c | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 6780f4d16e81..5e590504f3aa 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -462,6 +462,41 @@ static void ata_eh_clear_action(struct ata_link *link, struct ata_device *dev, } } +/** + * ata_eh_acquire - acquire EH ownership + * @ap: ATA port to acquire EH ownership for + * + * Acquire EH ownership for @ap. This is the basic exclusion + * mechanism for ports sharing a host. Only one port hanging off + * the same host can claim the ownership of EH. + * + * LOCKING: + * EH context. + */ +void ata_eh_acquire(struct ata_port *ap) +{ + mutex_lock(&ap->host->eh_mutex); + WARN_ON_ONCE(ap->host->eh_owner); + ap->host->eh_owner = current; +} + +/** + * ata_eh_release - release EH ownership + * @ap: ATA port to release EH ownership for + * + * Release EH ownership for @ap if the caller. The caller must + * have acquired EH ownership using ata_eh_acquire() previously. + * + * LOCKING: + * EH context. + */ +void ata_eh_release(struct ata_port *ap) +{ + WARN_ON_ONCE(ap->host->eh_owner != current); + ap->host->eh_owner = NULL; + mutex_unlock(&ap->host->eh_mutex); +} + /** * ata_scsi_timed_out - SCSI layer time out callback * @cmd: timed out SCSI command @@ -639,11 +674,13 @@ void ata_scsi_error(struct Scsi_Host *host) /* If we timed raced normal completion and there is nothing to recover nr_timedout == 0 why exactly are we doing error recovery ? */ - repeat: /* invoke error handler */ if (ap->ops->error_handler) { struct ata_link *link; + /* acquire EH ownership */ + ata_eh_acquire(ap); + repeat: /* kill fast drain timer */ del_timer_sync(&ap->fastdrain_timer); @@ -718,6 +755,7 @@ void ata_scsi_error(struct Scsi_Host *host) host->host_eh_scheduled = 0; spin_unlock_irqrestore(ap->lock, flags); + ata_eh_release(ap); } else { WARN_ON(ata_qc_from_tag(ap, ap->link.active_tag) == NULL); ap->ops->eng_timeout(ap); @@ -2818,8 +2856,10 @@ int ata_eh_reset(struct ata_link *link, int classify, "reset failed (errno=%d), retrying in %u secs\n", rc, DIV_ROUND_UP(jiffies_to_msecs(delta), 1000)); + ata_eh_release(ap); while (delta) delta = schedule_timeout_uninterruptible(delta); + ata_eh_acquire(ap); } if (try == max_tries - 1) { @@ -3635,8 +3675,10 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, if (time_before_eq(deadline, now)) break; + ata_eh_release(ap); deadline = wait_for_completion_timeout(&ap->park_req_pending, deadline - now); + ata_eh_acquire(ap); } while (deadline); ata_for_each_link(link, ap, EDGE) { ata_for_each_dev(dev, link, ALL) { -- cgit v1.2.2