From 1eca4365be25c540650693e941bc06a66cf38f94 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 3 Nov 2008 20:03:17 +0900 Subject: libata: beef up iterators There currently are the following looping constructs. * __ata_port_for_each_link() for all available links * ata_port_for_each_link() for edge links * ata_link_for_each_dev() for all devices * ata_link_for_each_dev_reverse() for all devices in reverse order Now there's a need for looping construct which is similar to __ata_port_for_each_link() but iterates over PMP links before the host link. Instead of adding another one with long name, do the following cleanup. * Implement and export ata_link_next() and ata_dev_next() which take @mode parameter and can be used to build custom loop. * Implement ata_for_each_link() and ata_for_each_dev() which take looping mode explicitly. The following iteration modes are implemented. * ATA_LITER_EDGE : loop over edge links * ATA_LITER_HOST_FIRST : loop over all links, host link first * ATA_LITER_PMP_FIRST : loop over all links, PMP links first * ATA_DITER_ENABLED : loop over enabled devices * ATA_DITER_ENABLED_REVERSE : loop over enabled devices in reverse order * ATA_DITER_ALL : loop over all devices * ATA_DITER_ALL_REVERSE : loop over all devices in reverse order This change removes exlicit device enabledness checks from many loops and makes it clear which ones are iterated over in which direction. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-eh.c | 84 ++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 47 deletions(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 32da9a93ce4..d673f3712bd 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -422,7 +422,7 @@ static void ata_eh_clear_action(struct ata_link *link, struct ata_device *dev, if (!dev) { ehi->action &= ~action; - ata_link_for_each_dev(tdev, link) + ata_for_each_dev(tdev, link, ALL) ehi->dev_action[tdev->devno] &= ~action; } else { /* doesn't make sense for port-wide EH actions */ @@ -430,7 +430,7 @@ static void ata_eh_clear_action(struct ata_link *link, struct ata_device *dev, /* break ehi->action into ehi->dev_action */ if (ehi->action & action) { - ata_link_for_each_dev(tdev, link) + ata_for_each_dev(tdev, link, ALL) ehi->dev_action[tdev->devno] |= ehi->action & action; ehi->action &= ~action; @@ -592,7 +592,7 @@ void ata_scsi_error(struct Scsi_Host *host) /* fetch & clear EH info */ spin_lock_irqsave(ap->lock, flags); - __ata_port_for_each_link(link, ap) { + ata_for_each_link(link, ap, HOST_FIRST) { struct ata_eh_context *ehc = &link->eh_context; struct ata_device *dev; @@ -600,12 +600,9 @@ void ata_scsi_error(struct Scsi_Host *host) link->eh_context.i = link->eh_info; memset(&link->eh_info, 0, sizeof(link->eh_info)); - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ENABLED) { int devno = dev->devno; - if (!ata_dev_enabled(dev)) - continue; - ehc->saved_xfer_mode[devno] = dev->xfer_mode; if (ata_ncq_enabled(dev)) ehc->saved_ncq_enabled |= 1 << devno; @@ -644,7 +641,7 @@ void ata_scsi_error(struct Scsi_Host *host) } /* this run is complete, make sure EH info is clear */ - __ata_port_for_each_link(link, ap) + ata_for_each_link(link, ap, HOST_FIRST) memset(&link->eh_info, 0, sizeof(link->eh_info)); /* Clear host_eh_scheduled while holding ap->lock such @@ -1025,7 +1022,7 @@ int sata_async_notification(struct ata_port *ap) struct ata_link *link; /* check and notify ATAPI AN */ - ata_port_for_each_link(link, ap) { + ata_for_each_link(link, ap, EDGE) { if (!(sntf & (1 << link->pmp))) continue; @@ -2005,7 +2002,7 @@ void ata_eh_autopsy(struct ata_port *ap) { struct ata_link *link; - ata_port_for_each_link(link, ap) + ata_for_each_link(link, ap, EDGE) ata_eh_link_autopsy(link); /* Handle the frigging slave link. Autopsy is done similarly @@ -2219,7 +2216,7 @@ void ata_eh_report(struct ata_port *ap) { struct ata_link *link; - __ata_port_for_each_link(link, ap) + ata_for_each_link(link, ap, HOST_FIRST) ata_eh_link_report(link); } @@ -2230,7 +2227,7 @@ static int ata_do_reset(struct ata_link *link, ata_reset_fn_t reset, struct ata_device *dev; if (clear_classes) - ata_link_for_each_dev(dev, link) + ata_for_each_dev(dev, link, ALL) classes[dev->devno] = ATA_DEV_UNKNOWN; return reset(link, classes, deadline); @@ -2294,7 +2291,7 @@ int ata_eh_reset(struct ata_link *link, int classify, ata_eh_about_to_do(link, NULL, ATA_EH_RESET); - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ALL) { /* If we issue an SRST then an ATA drive (not ATAPI) * may change configuration and be in PIO0 timing. If * we do a hard reset (or are coming from power on) @@ -2355,7 +2352,7 @@ int ata_eh_reset(struct ata_link *link, int classify, "port disabled. ignoring.\n"); ehc->i.action &= ~ATA_EH_RESET; - ata_link_for_each_dev(dev, link) + ata_for_each_dev(dev, link, ALL) classes[dev->devno] = ATA_DEV_NONE; rc = 0; @@ -2369,7 +2366,7 @@ int ata_eh_reset(struct ata_link *link, int classify, * bang classes and return. */ if (reset && !(ehc->i.action & ATA_EH_RESET)) { - ata_link_for_each_dev(dev, link) + ata_for_each_dev(dev, link, ALL) classes[dev->devno] = ATA_DEV_NONE; rc = 0; goto out; @@ -2454,7 +2451,7 @@ int ata_eh_reset(struct ata_link *link, int classify, /* * Post-reset processing */ - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ALL) { /* After the reset, the device state is PIO 0 and the * controller state is undefined. Reset also wakes up * drives from sleeping mode. @@ -2510,7 +2507,7 @@ int ata_eh_reset(struct ata_link *link, int classify, * can be reliably detected and retried. */ nr_unknown = 0; - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ALL) { /* convert all ATA_DEV_UNKNOWN to ATA_DEV_NONE */ if (classes[dev->devno] == ATA_DEV_UNKNOWN) { classes[dev->devno] = ATA_DEV_NONE; @@ -2619,8 +2616,8 @@ static inline void ata_eh_pull_park_action(struct ata_port *ap) spin_lock_irqsave(ap->lock, flags); INIT_COMPLETION(ap->park_req_pending); - ata_port_for_each_link(link, ap) { - ata_link_for_each_dev(dev, link) { + ata_for_each_link(link, ap, EDGE) { + ata_for_each_dev(dev, link, ALL) { struct ata_eh_info *ehi = &link->eh_info; link->eh_context.i.dev_action[dev->devno] |= @@ -2675,7 +2672,7 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link, * be done backwards such that PDIAG- is released by the slave * device before the master device is identified. */ - ata_link_for_each_dev_reverse(dev, link) { + ata_for_each_dev(dev, link, ALL_REVERSE) { unsigned int action = ata_eh_dev_action(dev); unsigned int readid_flags = 0; @@ -2744,7 +2741,7 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link, /* Configure new devices forward such that user doesn't see * device detection messages backwards. */ - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ALL) { if (!(new_mask & (1 << dev->devno)) || dev->class == ATA_DEV_PMP) continue; @@ -2793,10 +2790,7 @@ int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev) int rc; /* if data transfer is verified, clear DUBIOUS_XFER on ering top */ - ata_link_for_each_dev(dev, link) { - if (!ata_dev_enabled(dev)) - continue; - + ata_for_each_dev(dev, link, ENABLED) { if (!(dev->flags & ATA_DFLAG_DUBIOUS_XFER)) { struct ata_ering_entry *ent; @@ -2813,14 +2807,11 @@ int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev) rc = ata_do_set_mode(link, r_failed_dev); /* if transfer mode has changed, set DUBIOUS_XFER on device */ - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ENABLED) { struct ata_eh_context *ehc = &link->eh_context; u8 saved_xfer_mode = ehc->saved_xfer_mode[dev->devno]; u8 saved_ncq = !!(ehc->saved_ncq_enabled & (1 << dev->devno)); - if (!ata_dev_enabled(dev)) - continue; - if (dev->xfer_mode != saved_xfer_mode || ata_ncq_enabled(dev) != saved_ncq) dev->flags |= ATA_DFLAG_DUBIOUS_XFER; @@ -2881,9 +2872,8 @@ static int ata_link_nr_enabled(struct ata_link *link) struct ata_device *dev; int cnt = 0; - ata_link_for_each_dev(dev, link) - if (ata_dev_enabled(dev)) - cnt++; + ata_for_each_dev(dev, link, ENABLED) + cnt++; return cnt; } @@ -2892,7 +2882,7 @@ static int ata_link_nr_vacant(struct ata_link *link) struct ata_device *dev; int cnt = 0; - ata_link_for_each_dev(dev, link) + ata_for_each_dev(dev, link, ALL) if (dev->class == ATA_DEV_UNKNOWN) cnt++; return cnt; @@ -2918,7 +2908,7 @@ static int ata_eh_skip_recovery(struct ata_link *link) return 0; /* skip if class codes for all vacant slots are ATA_DEV_NONE */ - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ALL) { if (dev->class == ATA_DEV_UNKNOWN && ehc->classes[dev->devno] != ATA_DEV_NONE) return 0; @@ -3026,7 +3016,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, DPRINTK("ENTER\n"); /* prep for recovery */ - ata_port_for_each_link(link, ap) { + ata_for_each_link(link, ap, EDGE) { struct ata_eh_context *ehc = &link->eh_context; /* re-enable link? */ @@ -3038,7 +3028,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, ata_eh_done(link, NULL, ATA_EH_ENABLE_LINK); } - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ALL) { if (link->flags & ATA_LFLAG_NO_RETRY) ehc->tries[dev->devno] = 1; else @@ -3068,19 +3058,19 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, goto out; /* prep for EH */ - ata_port_for_each_link(link, ap) { + ata_for_each_link(link, ap, EDGE) { struct ata_eh_context *ehc = &link->eh_context; /* skip EH if possible. */ if (ata_eh_skip_recovery(link)) ehc->i.action = 0; - ata_link_for_each_dev(dev, link) + ata_for_each_dev(dev, link, ALL) ehc->classes[dev->devno] = ATA_DEV_UNKNOWN; } /* reset */ - ata_port_for_each_link(link, ap) { + ata_for_each_link(link, ap, EDGE) { struct ata_eh_context *ehc = &link->eh_context; if (!(ehc->i.action & ATA_EH_RESET)) @@ -3105,8 +3095,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, ata_eh_pull_park_action(ap); deadline = jiffies; - ata_port_for_each_link(link, ap) { - ata_link_for_each_dev(dev, link) { + ata_for_each_link(link, ap, EDGE) { + ata_for_each_dev(dev, link, ALL) { struct ata_eh_context *ehc = &link->eh_context; unsigned long tmp; @@ -3134,8 +3124,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, deadline = wait_for_completion_timeout(&ap->park_req_pending, deadline - now); } while (deadline); - ata_port_for_each_link(link, ap) { - ata_link_for_each_dev(dev, link) { + ata_for_each_link(link, ap, EDGE) { + ata_for_each_dev(dev, link, ALL) { if (!(link->eh_context.unloaded_mask & (1 << dev->devno))) continue; @@ -3146,7 +3136,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, } /* the rest */ - ata_port_for_each_link(link, ap) { + ata_for_each_link(link, ap, EDGE) { struct ata_eh_context *ehc = &link->eh_context; /* revalidate existing devices and attach new ones */ @@ -3172,7 +3162,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, * disrupting the current users of the device. */ if (ehc->i.flags & ATA_EHI_DID_RESET) { - ata_link_for_each_dev(dev, link) { + ata_for_each_dev(dev, link, ALL) { if (dev->class != ATA_DEV_ATAPI) continue; rc = atapi_eh_clear_ua(dev); @@ -3183,7 +3173,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, /* configure link power saving */ if (ehc->i.action & ATA_EH_LPM) - ata_link_for_each_dev(dev, link) + ata_for_each_dev(dev, link, ALL) ata_dev_enable_pm(dev, ap->pm_policy); /* this link is okay now */ @@ -3288,7 +3278,7 @@ void ata_do_eh(struct ata_port *ap, ata_prereset_fn_t prereset, rc = ata_eh_recover(ap, prereset, softreset, hardreset, postreset, NULL); if (rc) { - ata_link_for_each_dev(dev, &ap->link) + ata_for_each_dev(dev, &ap->link, ALL) ata_dev_disable(dev); } -- cgit v1.2.3-70-g09d2 From ece180d1cfe5fa751eaa85bf796cf28b2150af15 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 3 Nov 2008 20:04:37 +0900 Subject: libata: perform port detach in EH ata_port_detach() first made sure EH saw ATA_PFLAG_UNLOADING and then assumed EH context belongs to it and performed detach operation itself. However, UNLOADING doesn't disable all of EH and this could lead to problems including triggering WARN_ON()'s in EH path. This patch makes port detach behave more like other EH actions such that ata_port_detach() requests EH to detach and waits for completion. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 23 ++++------------------- drivers/ata/libata-eh.c | 32 +++++++++++++++++++++++++++++++- include/linux/libata.h | 3 ++- 3 files changed, 37 insertions(+), 21 deletions(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 1ecc3cb0b72..837fb60a6dc 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6107,8 +6107,6 @@ int ata_host_activate(struct ata_host *host, int irq, static void ata_port_detach(struct ata_port *ap) { unsigned long flags; - struct ata_link *link; - struct ata_device *dev; if (!ap->ops->error_handler) goto skip_eh; @@ -6116,28 +6114,15 @@ static void ata_port_detach(struct ata_port *ap) /* tell EH we're leaving & flush EH */ spin_lock_irqsave(ap->lock, flags); ap->pflags |= ATA_PFLAG_UNLOADING; + ata_port_schedule_eh(ap); spin_unlock_irqrestore(ap->lock, flags); + /* wait till EH commits suicide */ ata_port_wait_eh(ap); - /* EH is now guaranteed to see UNLOADING - EH context belongs - * to us. Restore SControl and disable all existing devices. - */ - ata_for_each_link(link, ap, PMP_FIRST) { - sata_scr_write(link, SCR_CONTROL, link->saved_scontrol & 0xff0); - ata_for_each_dev(dev, link, ALL) - ata_dev_disable(dev); - } - - /* Final freeze & EH. All in-flight commands are aborted. EH - * will be skipped and retrials will be terminated with bad - * target. - */ - spin_lock_irqsave(ap->lock, flags); - ata_port_freeze(ap); /* won't be thawed */ - spin_unlock_irqrestore(ap->lock, flags); + /* it better be dead now */ + WARN_ON(!(ap->pflags & ATA_PFLAG_UNLOADED)); - ata_port_wait_eh(ap); cancel_rearming_delayed_work(&ap->hotplug_task); skip_eh: diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index d673f3712bd..8147a838637 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -491,6 +491,31 @@ enum blk_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd) return ret; } +static void ata_eh_unload(struct ata_port *ap) +{ + struct ata_link *link; + struct ata_device *dev; + unsigned long flags; + + /* Restore SControl IPM and SPD for the next driver and + * disable attached devices. + */ + ata_for_each_link(link, ap, PMP_FIRST) { + sata_scr_write(link, SCR_CONTROL, link->saved_scontrol & 0xff0); + ata_for_each_dev(dev, link, ALL) + ata_dev_disable(dev); + } + + /* freeze and set UNLOADED */ + spin_lock_irqsave(ap->lock, flags); + + ata_port_freeze(ap); /* won't be thawed */ + ap->pflags &= ~ATA_PFLAG_EH_PENDING; /* clear pending from freeze */ + ap->pflags |= ATA_PFLAG_UNLOADED; + + spin_unlock_irqrestore(ap->lock, flags); +} + /** * ata_scsi_error - SCSI layer error handler callback * @host: SCSI host on which error occurred @@ -618,8 +643,13 @@ void ata_scsi_error(struct Scsi_Host *host) /* invoke EH, skip if unloading or suspended */ if (!(ap->pflags & (ATA_PFLAG_UNLOADING | ATA_PFLAG_SUSPENDED))) ap->ops->error_handler(ap); - else + else { + /* if unloading, commence suicide */ + if ((ap->pflags & ATA_PFLAG_UNLOADING) && + !(ap->pflags & ATA_PFLAG_UNLOADED)) + ata_eh_unload(ap); ata_eh_finish(ap); + } /* process port suspend request */ ata_eh_handle_port_suspend(ap); diff --git a/include/linux/libata.h b/include/linux/libata.h index 3b2a0c6444e..3449de597ef 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -213,10 +213,11 @@ enum { ATA_PFLAG_FROZEN = (1 << 2), /* port is frozen */ ATA_PFLAG_RECOVERED = (1 << 3), /* recovery action performed */ ATA_PFLAG_LOADING = (1 << 4), /* boot/loading probe */ - ATA_PFLAG_UNLOADING = (1 << 5), /* module is unloading */ ATA_PFLAG_SCSI_HOTPLUG = (1 << 6), /* SCSI hotplug scheduled */ ATA_PFLAG_INITIALIZING = (1 << 7), /* being initialized, don't touch */ ATA_PFLAG_RESETTING = (1 << 8), /* reset in progress */ + ATA_PFLAG_UNLOADING = (1 << 9), /* driver is being unloaded */ + ATA_PFLAG_UNLOADED = (1 << 10), /* driver is unloaded */ ATA_PFLAG_SUSPENDED = (1 << 17), /* port is suspended (power) */ ATA_PFLAG_PM_PENDING = (1 << 18), /* PM operation pending */ -- cgit v1.2.3-70-g09d2