From: Tejun Heo <tj@kernel.org>
To: Tim Connors <tconnors+linuxkml@astro.swin.edu.au>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-ide@vger.kernel.org, Jeff Garzik <jeff@garzik.org>
Subject: [PATCH #upstream-fixes] libata: fix attach error handling
Date: Sun, 10 May 2009 01:06:54 +0900 [thread overview]
Message-ID: <4A05AA1E.3070700@kernel.org> (raw)
In-Reply-To: <4A04DF32.4050305@kernel.org>
New device attach path in ata_eh_revalidate_and_attach() is divided
into two separate loops because ATA requires IDENTIFY to be issued to
slave first while the user expects to see device probe messages from
the master device. new_mask is used to track which devices are the
new ones between the first loop and the second.
This usually works well but if an error occurs during configuration
stage, ata_dev_revalidate_and_attach() returns with error code and
forgets new_mask. On the retry run, dev->class is set and new_mask
for the device is clear, so the device just gets revalidated and thus
ends up skipping post-configuration procedure including scheduling of
SCSI_HOTPLUG for the device. When this occurs, ATA part of probing
works fine but SCSI probing usually doesn't happen and makes the
device unreachable.
The behavior has been around for a very long time but it has been
uncovered with the recent addition of 1_5_GBPS horkage which uses
-EAGAIN return value from ata_dev_configure() to restart the probing
sequence after forcing cable speed.
This can be fixed by making sure dev->class is permanently set only
after all configurations are successfully complete. Fix it.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Tim Connors <tconnors+linuxkml@astro.swin.edu.au>
---
drivers/ata/libata-eh.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 0183131..eb8b940 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2783,6 +2783,12 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
} else if (dev->class == ATA_DEV_UNKNOWN &&
ehc->tries[dev->devno] &&
ata_class_enabled(ehc->classes[dev->devno])) {
+ /* Temporarily set dev->class, it will be
+ * permanently set once all configurations are
+ * complete. This is necessary because new
+ * device configuration is done in two
+ * separate loops.
+ */
dev->class = ehc->classes[dev->devno];
if (dev->class == ATA_DEV_PMP)
@@ -2790,6 +2796,11 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
else
rc = ata_dev_read_id(dev, &dev->class,
readid_flags, dev->id);
+
+ /* read_id might have changed class, store and reset */
+ ehc->classes[dev->devno] = dev->class;
+ dev->class = ATA_DEV_UNKNOWN;
+
switch (rc) {
case 0:
/* clear error info accumulated during probe */
@@ -2799,13 +2810,11 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
case -ENOENT:
/* IDENTIFY was issued to non-existent
* device. No need to reset. Just
- * thaw and kill the device.
+ * thaw and ignore the device.
*/
ata_eh_thaw_port(ap);
- dev->class = ATA_DEV_UNKNOWN;
break;
default:
- dev->class = ATA_DEV_UNKNOWN;
goto err;
}
}
@@ -2826,11 +2835,15 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
dev->class == ATA_DEV_PMP)
continue;
+ dev->class = ehc->classes[dev->devno];
+
ehc->i.flags |= ATA_EHI_PRINTINFO;
rc = ata_dev_configure(dev);
ehc->i.flags &= ~ATA_EHI_PRINTINFO;
- if (rc)
+ if (rc) {
+ dev->class = ATA_DEV_UNKNOWN;
goto err;
+ }
spin_lock_irqsave(ap->lock, flags);
ap->pflags |= ATA_PFLAG_SCSI_HOTPLUG;
next prev parent reply other threads:[~2009-05-09 16:10 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-05 14:22 sata_sil24 EH complete Tim Connors
2009-05-06 0:32 ` Tejun Heo
2009-05-06 0:38 ` Tim Connors
2009-05-08 14:11 ` Tim Connors
2009-05-09 1:41 ` Tejun Heo
2009-05-09 16:06 ` Tejun Heo [this message]
2009-05-11 18:33 ` [PATCH #upstream-fixes] libata: fix attach error handling Jeff Garzik
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=4A05AA1E.3070700@kernel.org \
--to=tj@kernel.org \
--cc=jeff@garzik.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tconnors+linuxkml@astro.swin.edu.au \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).