Skip to contentSkip to conversation

ata: acpi: make ata_ap_acpi_handle not block

PATCH In review Aaron Lu 2 participants 3 messages Apr 27, 2013 – Apr 30, 2013

Subsystems (2)
lore
Share & tools
lore

Aaron Lu

author
#0 lore raw

PATCH 1/2 ata: acpi: make ata_ap_acpi_handle not block

Since commit 30dcf76acc, ata_ap_acpi_handle will always do a namespace walk, which requires acquiring an acpi namespace mutex. This made it impossible to be used when calling path has held a spinlock.

For example, it can occur in the following code path for pata_acpi:
ata_scsi_queuecmd (ap->lock is acquired)
__ata_scsi_queuecmd
ata_scsi_translate
ata_qc_issue
pacpi_qc_issue
ata_acpi_stm
ata_ap_acpi_handle
acpi_get_child
acpi_walk_namespace
acpi_ut_acquire_mutex (acquire mutex while holding lock)
This caused scheduling while atomic bug, as reported in bug #56781.

Actually, ata_ap_acpi_handle doesn't have to walk the namespace every time it is called, it can simply return the bound acpi handle on the corresponding SCSI host. The reason previously it is not done this way is, ata_ap_acpi_handle is used in the binding function ata_acpi_bind_host by ata_acpi_gtm when the handle is not bound to the SCSI host yet. Since we already have the ATA port's handle in its binding function, we can simply use it instead of calling ata_ap_acpi_handle there. So introduce a new function __ata_acpi_gtm, where it will receive an acpi handle param in addition to the ATA port which is solely used for debug statement. With this change, we can make ata_ap_acpi_handle simply return the bound handle for SCSI host instead of walking the acpi namespace now.

---
 drivers/ata/libata-acpi.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)
drivers/ata/libata-acpi.c +27 -18
index 1c33f78..f6d80e3 100644
@@ -60,7 +60,8 @@ acpi_handle ata_ap_acpi_handle(struct ata_port *ap)
 	if (ap->flags & ATA_FLAG_ACPI_SATA)
 		return NULL;
 
-	return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev), ap->port_no);
+	return ap->scsi_host ?
+		DEVICE_ACPI_HANDLE(&ap->scsi_host->shost_gendev) : NULL;
 }
 EXPORT_SYMBOL(ata_ap_acpi_handle);
 
@@ -239,28 +240,15 @@ void ata_acpi_dissociate(struct ata_host *host)
 	}
 }
 
-/**
- * ata_acpi_gtm - execute _GTM
- * @ap: target ATA port
- * @gtm: out parameter for _GTM result
- *
- * Evaluate _GTM and store the result in @gtm.
- *
- * LOCKING:
- * EH context.
- *
- * RETURNS:
- * 0 on success, -ENOENT if _GTM doesn't exist, -errno on failure.
- */
-int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *gtm)
+static int __ata_acpi_gtm(struct ata_port *ap, acpi_handle handle,
+			  struct ata_acpi_gtm *gtm)
 {
 	struct acpi_buffer output = { .length = ACPI_ALLOCATE_BUFFER };
 	union acpi_object *out_obj;
 	acpi_status status;
 	int rc = 0;
 
-	status = acpi_evaluate_object(ata_ap_acpi_handle(ap), "_GTM", NULL,
-				      &output);
+	status = acpi_evaluate_object(handle, "_GTM", NULL, &output);
 
 	rc = -ENOENT;
 	if (status == AE_NOT_FOUND)
@@ -294,6 +282,27 @@ int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *gtm)
 	return rc;
 }
 
+/**
+ * ata_acpi_gtm - execute _GTM
+ * @ap: target ATA port
+ * @gtm: out parameter for _GTM result
+ *
+ * Evaluate _GTM and store the result in @gtm.
+ *
+ * LOCKING:
+ * EH context.
+ *
+ * RETURNS:
+ * 0 on success, -ENOENT if _GTM doesn't exist, -errno on failure.
+ */
+int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *gtm)
+{
+	if (ata_ap_acpi_handle(ap))
+		return __ata_acpi_gtm(ap, ata_ap_acpi_handle(ap), gtm);
+	else
+		return -EINVAL;
+}
+
 EXPORT_SYMBOL_GPL(ata_acpi_gtm);
 
 /**
@@ -1047,7 +1056,7 @@ static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
 	if (!*handle)
 		return -ENODEV;
 
-	if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
+	if (__ata_acpi_gtm(ap, *handle, &ap->__acpi_init_gtm) == 0)
 		ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
 
 	return 0;
Signature · 1 line
1.8.2.1.94.g173f9a7
author
#1 lore raw

PATCH 2/2 ACPI/libata: Restore libata.noacpi support

From: Lv Zheng <[email protected]>

This patch restores libata.noacpi support to libata-acpi.c. There are broken optional control methods for ATA controller devices in the real world. The libata.noacpi has been used for a long time as a workaround to deal with issues caused by the broken ASL codes.
1. The "noacpi" option is introduced by the following commit:
commit 11ef697b37e3c85ce1ac21f7711babf1f5b12784
Date: Thu, 28 Sep 2006 11:29:01 -0700
Subject: libata: ACPI and _GTF support
2. The "noacpi" option is renamed to "libata_noacpi" by the following commit:
commit d7d0dad62a641c156386288a747c1a2f6bb2e42d
Date: Wed, 28 Mar 2007 01:57:37 -0400
Subject: [libata] Disable ACPI by default; fix namespace problems
3. Some of its logics are changed over time - becomes relying on the "acpi_handle" bound to the ATA devices since this commit:
commit fafbae87db88a73b166d3bc3294d209207f27056
Date: Tue, 15 May 2007 03:28:16 +0900
Subject: libata-acpi: implement ata_acpi_associate()
4. The option is deleted by the following commit:
commit 30dcf76acc695cbd2fa919e294670fe9552e16e7
Date: Mon, 25 Jun 2012 16:13:04 +0800
Subject: libata: migrate ACPI code over to new bindings
But the libata.noacpi setup is still left in the kernel without codes to implement it. So the deletion introduces a regression to the Linux. This patch disables ATA_ACPI support at runtime by stopping acpi binding on the ATA devices to fix this regression.
This patch is tested by booting a SATA x86-64 kernel or a PATA x86 kernel with or without "libata.noacpi=1" kernel command line argument.

---
 drivers/ata/libata-acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/ata/libata-acpi.c +2 -2
index f6d80e3..87f2f39 100644
@@ -77,7 +77,7 @@ acpi_handle ata_dev_acpi_handle(struct ata_device *dev)
 	acpi_integer adr;
 	struct ata_port *ap = dev->link->ap;
 
-	if (dev->flags & ATA_DFLAG_ACPI_DISABLED)
+	if (libata_noacpi || dev->flags & ATA_DFLAG_ACPI_DISABLED)
 		return NULL;
 
 	if (ap->flags & ATA_FLAG_ACPI_SATA) {
@@ -1047,7 +1047,7 @@ static int compat_pci_ata(struct ata_port *ap)
 
 static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
 {
-	if (ap->flags & ATA_FLAG_ACPI_SATA)
+	if (libata_noacpi || ap->flags & ATA_FLAG_ACPI_SATA)
 		return -ENODEV;
 
 	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(ap->tdev.parent),
Signature · 1 line
1.8.2.1.94.g173f9a7