Bug 10752

Summary: pata_isapnp driver causes the ata layer to go splat
Product: IO/Storage Reporter: Arjan van de Ven (arjan)
Component: Serial ATAAssignee: Alan (alan)
Status: CLOSED CODE_FIX    
Severity: normal CC: alan
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.25 Subsystem:
Regression: --- Bisected commit-id:

Description Arjan van de Ven 2008-05-19 14:46:08 UTC
This happens quite a bit based on Fedora 9 feedback, but already happened in 2.6.24 and possibly earlier.

http://www.kerneloops.org/search.php?search=bad_io_access

shows the basic backtrace:
*bad_io_access
ioread8
ata_altstatus
ata_std_dev_select
ata_dev_select
ata_qc_issue_prot
ata_qc_issue
ata_exec_internal_sg
ata_exec_internal
ata_dev_read_id
ata_eh_recover
ata_do_eh
ata_bmdma_drive_eh
ata_bmdma_error_handler
ata_scsi_error
scsi_error_handler
kthread
kernel_thread_helper

in all cases, this happens while pata_isapnp is being loaded; so it's not unlikely that this driver stomps on some other driver which then goes splat
Comment 1 Anonymous Emailer 2008-05-19 15:26:30 UTC
Reply-To: akpm@linux-foundation.org

On Mon, 19 May 2008 14:46:08 -0700 (PDT)
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=10752
> 
>            Summary: pata_isapnp driver causes the ata layer to go splat
>            Product: IO/Storage
>            Version: 2.5
>      KernelVersion: 2.6.25
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Serial ATA
>         AssignedTo: jgarzik@pobox.com
>         ReportedBy: arjan@linux.intel.com
> 
> 
> This happens quite a bit based on Fedora 9 feedback, but already happened in
> 2.6.24 and possibly earlier.
> 
> http://www.kerneloops.org/search.php?search=bad_io_access
> 
> shows the basic backtrace:
> *bad_io_access
> ioread8
> ata_altstatus
> ata_std_dev_select
> ata_dev_select
> ata_qc_issue_prot
> ata_qc_issue
> ata_exec_internal_sg
> ata_exec_internal
> ata_dev_read_id
> ata_eh_recover
> ata_do_eh
> ata_bmdma_drive_eh
> ata_bmdma_error_handler
> ata_scsi_error
> scsi_error_handler
> kthread
> kernel_thread_helper
> 
> in all cases, this happens while pata_isapnp is being loaded; so it's not
> unlikely that this driver stomps on some other driver which then goes splat
> 

Unclear who maintains pata_isapnp?
Comment 2 Arjan van de Ven 2008-05-19 18:13:42 UTC
On Mon, 19 May 2008 15:25:49 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> > 
> 
> Unclear who maintains pata_isapnp?

Alan is on top of this one; he sent a patch to Jeff Garzik already
earlier today
(will attach to the BZ shortly)
Comment 3 Arjan van de Ven 2008-05-19 18:48:13 UTC
libata-sff: Fix oops reported in kerneloops.org for pnp devices with no ctl

From: Alan Cox <alan@redhat.com>


---

 drivers/ata/libata-sff.c |   23 +++++++++++++++++++++++
 include/linux/libata.h   |   14 +-------------
 2 files changed, 24 insertions(+), 13 deletions(-)


diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 3c2d228..dbea523 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -256,6 +256,28 @@ u8 ata_sff_altstatus(struct ata_port *ap)
 }
 
 /**
+ *	ata_sff_pause - Flush writes and pause 400 nanoseconds.
+ *	@ap: Port to wait for.
+ *
+ *	CAUTION:
+ *	If we have an mmio device with no ctl and no altstatus
+ *	method this will fail. No such devices are known to exist.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+
+void ata_sff_pause(struct ata_port *ap)
+{
+	if (ap->ops->sff_check_altstatus)
+		ap->ops->sff_check_altstatus(ap);
+	else if (ap->ioaddr.altstatus_addr)
+		ioread8(ap->ioaddr.altstatus_addr);
+	ndelay(400);
+}
+
+
+/**
  *	ata_sff_busy_sleep - sleep until BSY clears, or timeout
  *	@ap: port containing status register to be polled
  *	@tmout_pat: impatience timeout
@@ -2723,6 +2745,7 @@ EXPORT_SYMBOL_GPL(ata_sff_dumb_qc_prep);
 EXPORT_SYMBOL_GPL(ata_sff_dev_select);
 EXPORT_SYMBOL_GPL(ata_sff_check_status);
 EXPORT_SYMBOL_GPL(ata_sff_altstatus);
+EXPORT_SYMBOL_GPL(ata_sff_pause);
 EXPORT_SYMBOL_GPL(ata_sff_busy_sleep);
 EXPORT_SYMBOL_GPL(ata_sff_wait_ready);
 EXPORT_SYMBOL_GPL(ata_sff_tf_load);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0f17643..6a94bec 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1437,6 +1437,7 @@ extern void ata_sff_qc_prep(struct ata_queued_cmd *qc);
 extern void ata_sff_dumb_qc_prep(struct ata_queued_cmd *qc);
 extern void ata_sff_dev_select(struct ata_port *ap, unsigned int device);
 extern u8 ata_sff_check_status(struct ata_port *ap);
+extern void ata_sff_pause(struct ata_port *ap);
 extern u8 ata_sff_altstatus(struct ata_port *ap);
 extern int ata_sff_busy_sleep(struct ata_port *ap,
 			      unsigned long timeout_pat, unsigned long timeout);
@@ -1498,19 +1499,6 @@ extern int ata_pci_sff_init_one(struct pci_dev *pdev,
 #endif /* CONFIG_PCI */
 
 /**
- *	ata_sff_pause - Flush writes and pause 400 nanoseconds.
- *	@ap: Port to wait for.
- *
- *	LOCKING:
- *	Inherited from caller.
- */
-static inline void ata_sff_pause(struct ata_port *ap)
-{
-	ata_sff_altstatus(ap);
-	ndelay(400);
-}
-
-/**
  *	ata_sff_busy_wait - Wait for a port status register
  *	@ap: Port to wait for.
  *	@bits: bits that must be clear