Skip to contentSkip to conversation

libata: make EH fail gracefully if no reset method is available

PATCH Applied Tejun Heo 2 participants 5 messages Jan 30, 2008 – Apr 12, 2008 libata

lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 libata: make EH fail gracefully if no reset method is available

When no reset method is available, libata currently oopses. Although the condition can't happen unless there's a bug in a low level driver, oopsing isn't the best way to report the error condition. Complain, dump stack and fail reset instead.

Signed-off-by Tejun Heo <[email protected]>

---
This patch is on top of

upstream (a984f58dd97f22f5113700322fed311a0ee29947)
+ prefer-hardreset patchset
http://thread.gmane.org/gmane.linux.ide/27447

 drivers/ata/libata-eh.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
drivers/ata/libata-eh.c +6 -1
index 64418f1..8d82b06 100644
@@ -2135,9 +2135,14 @@ int ata_eh_reset(struct ata_link *link, int classify,
 	if (hardreset) {
 		reset = hardreset;
 		ehc->i.action = ATA_EH_HARDRESET;
-	} else {
+	} else if (softreset) {
 		reset = softreset;
 		ehc->i.action = ATA_EH_SOFTRESET;
+	} else {
+		ata_link_printk(link, KERN_ERR, "BUG: no reset method, "
+				"please report to [email protected]\n");
+		dump_stack();
+		return -EINVAL;
 	}
 
 	if (prereset) {
Signature · 1 line
1.5.2.4
#1 lore raw
Quoted 9 lines · Tejun Heo

When no reset method is available, libata currently oopses. Although the condition can't happen unless there's a bug in a low level driver, oopsing isn't the best way to report the error condition. Complain, dump stack and fail reset instead.

Signed-off-by Tejun Heo <[email protected]>

---
This patch is on top of

Quoted 6 lines

upstream (a984f58dd97f22f5113700322fed311a0ee29947)
+ prefer-hardreset patchset
http://thread.gmane.org/gmane.linux.ide/27447

 drivers/ata/libata-eh.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

ACK (held in mbox, due to waiting on drive manufacturers' responses)

#2 lore raw
In reply to Tejun Heo
Quoted 23 lines · Tejun Heo

When no reset method is available, libata currently oopses. Although the condition can't happen unless there's a bug in a low level driver, oopsing isn't the best way to report the error condition. Complain, dump stack and fail reset instead.

Signed-off-by Tejun Heo <[email protected]>

---
This patch is on top of

upstream (a984f58dd97f22f5113700322fed311a0ee29947)
+ prefer-hardreset patchset
http://thread.gmane.org/gmane.linux.ide/27447

 drivers/ata/libata-eh.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
drivers/ata/libata-eh.c +0 -0
index 64418f1..8d82b06 100644
@@ -2135,9 +2135,14 @@ int ata_eh_reset(struct ata_link *link, int classify,
 	if (hardreset) {
 		reset = hardreset;

drivers/ata/libata-eh.c int ata_eh_reset(struct ata_link *link, int classify, · line 2145

Quoted 10 lines
drivers/ata/libata-eh.c +6 -1
 		ehc->i.action = ATA_EH_HARDRESET;
-	} else {
+	} else if (softreset) {
 		reset = softreset;
 		ehc->i.action = ATA_EH_SOFTRESET;
+	} else {
+		ata_link_printk(link, KERN_ERR, "BUG: no reset method, "
+				"please report to [email protected]\n");
+		dump_stack();
+		return -EINVAL;

ACK, but holding due to hold on prefer-hardreset patch (pinging device manufacturers)

author
#3 lore raw
Quoted 30 lines · Jeff Garzik
Quoted 29 lines · Tejun Heo

When no reset method is available, libata currently oopses. Although the condition can't happen unless there's a bug in a low level driver, oopsing isn't the best way to report the error condition. Complain, dump stack and fail reset instead.

Signed-off-by Tejun Heo <[email protected]>

---
This patch is on top of

upstream (a984f58dd97f22f5113700322fed311a0ee29947)
+ prefer-hardreset patchset
http://thread.gmane.org/gmane.linux.ide/27447

 drivers/ata/libata-eh.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
drivers/ata/libata-eh.c +1 -1
index 64418f1..8d82b06 100644
@@ -2135,9 +2135,14 @@ int ata_eh_reset(struct ata_link *link, int
 classify,
     if (hardreset) {
         reset = hardreset;
         ehc->i.action = ATA_EH_HARDRESET;
-    } else {
+    } else if (softreset) {
         reset = softreset;
         ehc->i.action = ATA_EH_SOFTRESET;
Quoted 8 lines

drivers/ata/libata-eh.c int ata_eh_reset(struct ata_link *link, int classify, · line 2145

Quoted 5 lines
drivers/ata/libata-eh.c +5 -0
+    } else {
+        ata_link_printk(link, KERN_ERR, "BUG: no reset method, "
+                "please report to [email protected]\n");
+        dump_stack();
+        return -EINVAL;

ACK, but holding due to hold on prefer-hardreset patch (pinging device manufacturers)

Please drop this one. #upstream-fixes now has a patch to allow drivers w/o any reset method.

Signature · 1 line
tejun
Applied
#4 lore raw
In reply to Tejun Heo
Quoted 9 lines · Tejun Heo

When no reset method is available, libata currently oopses. Although the condition can't happen unless there's a bug in a low level driver, oopsing isn't the best way to report the error condition. Complain, dump stack and fail reset instead.

Signed-off-by Tejun Heo <[email protected]>

---
This patch is on top of

Quoted 6 lines

upstream (a984f58dd97f22f5113700322fed311a0ee29947)
+ prefer-hardreset patchset
http://thread.gmane.org/gmane.linux.ide/27447

 drivers/ata/libata-eh.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

applied