All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
To: Alex Chiang <achiang@hp.com>, Greg KH <gregkh@suse.de>
Cc: Gary Hade <garyhade@us.ibm.com>,
	Kristen Carlson Accardi <kristen.c.accardi@intel.com>,
	Matthew Wilcox <matthew@wil.cx>,
	warthog19@eaglescrag.net, rick.jones2@hp.com,
	linux-kernel@vger.kernel.org, linux-pci@atrey.karlin.mff.cuni.cz,
	linux-acpi@vger.kernel.org
Subject: [PATCH  6/16][BUG] ACPI pci_slot: Fix slot removal path (Not for mainline!)
Date: Fri, 21 Mar 2008 13:14:03 +0900	[thread overview]
Message-ID: <47E3360B.1010506@jp.fujitsu.com> (raw)
In-Reply-To: <47E33472.1000602@jp.fujitsu.com>

Current ACPI pci slot driver scans ACPI namespace and slot list on
pci_bus structure to find the pci_slot pointer. It is definitely
redundant. In addition, it scans slot list on pci_bus without holding
pci_bus_sem. This patch fixes those problems.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/acpi/pci_slot.c |  104 +++++++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 49 deletions(-)

Index: linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================
--- linux-2.6.25-rc6.orig/drivers/acpi/pci_slot.c
+++ linux-2.6.25-rc6/drivers/acpi/pci_slot.c
@@ -55,9 +55,17 @@ ACPI_MODULE_NAME("pci_slot");
 				MY_NAME , ## arg);		\
 	} while (0)
 
+struct acpi_pci_slot {
+	acpi_handle root_handle;	/* handle of the root bridge */
+	struct pci_slot *pci_slot;	/* corresponding pci_slot */
+	struct list_head list;		/* node in the list of slots */
+};
+
 static int acpi_pci_slot_add(acpi_handle handle);
 static void acpi_pci_slot_remove(acpi_handle handle);
 
+static LIST_HEAD(slot_list);
+static DEFINE_MUTEX(slot_list_lock);
 static struct acpi_pci_driver acpi_pci_slot_driver = {
 	.add = acpi_pci_slot_add,
 	.remove = acpi_pci_slot_remove,
@@ -105,34 +113,11 @@ out:
 	return retval;
 }
 
-/*
- * unregister_slot
- *
- * Called once for each SxFy object in the namespace. Each call to
- * pci_destroy_slot decrements the refcount on the pci_slot, and
- * eventually calls kobject_unregister at the appropriate time.
- */
-static acpi_status
-unregister_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	int device;
-	unsigned long sun;
-	struct pci_slot *slot, *tmp;
-	struct pci_bus *pci_bus = context;
-
-	if (check_slot(handle, &device, &sun))
-		return AE_OK;
-
-	/* FIXME - Need pci_bus_sem to be held */
-	list_for_each_entry_safe(slot, tmp, &pci_bus->slots, list) {
-		if (slot->number == device) {
-			pci_destroy_slot(slot);
-			break;
-		}
-	}
-
-	return AE_OK;
-}
+struct callback_args {
+	acpi_walk_callback	user_function;	/* only for walk_p2p_bridge */
+	struct pci_bus		*pci_bus;
+	acpi_handle		root_handle;
+};
 
 /*
  * register_slot
@@ -150,17 +135,33 @@ register_slot(acpi_handle handle, u32 lv
 	int device;
 	unsigned long sun;
 	char name[KOBJ_NAME_LEN];
-
+	struct acpi_pci_slot *slot;
 	struct pci_slot *pci_slot;
-	struct pci_bus *pci_bus = context;
+	struct callback_args *parent_context = context;
+	struct pci_bus *pci_bus = parent_context->pci_bus;
 
 	if (check_slot(handle, &device, &sun))
 		return AE_OK;
 
+	slot = kmalloc(sizeof(*slot), GFP_KERNEL);
+	if (!slot) {
+		err("%s: cannot allocate memory\n", __FUNCTION__);
+		return AE_OK;
+	}
+
 	snprintf(name, sizeof(name), "%u", (u32)sun);
 	pci_slot = pci_create_slot(pci_bus, device, name);
-	if (IS_ERR(pci_slot))
+	if (IS_ERR(pci_slot)) {
 		err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
+		kfree(slot);
+	}
+
+	slot->root_handle = parent_context->root_handle;
+	slot->pci_slot = pci_slot;
+	INIT_LIST_HEAD(&slot->list);
+	mutex_lock(&slot_list_lock);
+	list_add(&slot->list, &slot_list);
+	mutex_unlock(&slot_list_lock);
 
 	dbg("pci_slot: %#lx, pci_bus: %x, device: %d, name: %s\n",
 		(uint64_t)pci_slot, pci_bus->number, device, name);
@@ -168,11 +169,6 @@ register_slot(acpi_handle handle, u32 lv
 	return AE_OK;
 }
 
-struct p2p_bridge_context {
-	acpi_walk_callback	user_function;
-	struct pci_bus		*pci_bus;
-};
-
 /*
  * walk_p2p_bridge - discover and walk p2p bridges
  * @handle: points to an acpi_pci_root
@@ -192,8 +188,8 @@ walk_p2p_bridge(acpi_handle handle, u32 
 
 	struct pci_dev *dev;
 	struct pci_bus *pci_bus;
-	struct p2p_bridge_context child_context;
-	struct p2p_bridge_context *parent_context = context;
+	struct callback_args child_context;
+	struct callback_args *parent_context = context;
 
 	pci_bus = parent_context->pci_bus;
 	user_function = parent_context->user_function;
@@ -213,14 +209,16 @@ walk_p2p_bridge(acpi_handle handle, u32 
 	if (!dev || !dev->subordinate)
 		goto out;
 
+	child_context.pci_bus = dev->subordinate;
+	child_context.user_function = user_function;
+	child_context.root_handle = parent_context->root_handle;
+
 	dbg("p2p bridge walk, pci_bus = %x\n", dev->subordinate->number);
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
-				     user_function, dev->subordinate, NULL);
+				     user_function, &child_context, NULL);
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	child_context.pci_bus = dev->subordinate;
-	child_context.user_function = user_function;
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
 				     walk_p2p_bridge, &child_context, NULL);
 out:
@@ -246,7 +244,7 @@ walk_root_bridge(acpi_handle handle, acp
 	acpi_status status;
 	acpi_handle dummy_handle;
 	struct pci_bus *pci_bus;
-	struct p2p_bridge_context context;
+	struct callback_args context;
 
 	/* If the bridge doesn't have _STA, we assume it is always there */
 	status = acpi_get_handle(handle, "_STA", &dummy_handle);
@@ -271,14 +269,16 @@ walk_root_bridge(acpi_handle handle, acp
 	if (!pci_bus)
 		return 0;
 
+	context.pci_bus = pci_bus;
+	context.user_function = user_function;
+	context.root_handle = handle;
+
 	dbg("root bridge walk, pci_bus = %x\n", pci_bus->number);
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
-				     user_function, pci_bus, NULL);
+				     user_function, &context, NULL);
 	if (ACPI_FAILURE(status))
 		return status;
 
-	context.pci_bus = pci_bus;
-	context.user_function = user_function;
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
 				     walk_p2p_bridge, &context, NULL);
 	if (ACPI_FAILURE(status))
@@ -310,11 +310,17 @@ acpi_pci_slot_add(acpi_handle handle)
 static void
 acpi_pci_slot_remove(acpi_handle handle)
 {
-	acpi_status status;
+	struct acpi_pci_slot *slot, *tmp;
 
-	status = walk_root_bridge(handle, unregister_slot);
-	if (ACPI_FAILURE(status))
-		err("%s: unregister_slot failure - %d\n", __func__, status);
+	mutex_lock(&slot_list_lock);
+	list_for_each_entry_safe(slot, tmp, &slot_list, list) {
+		if (slot->root_handle == handle) {
+			list_del(&slot->list);
+			pci_destroy_slot(slot->pci_slot);
+			kfree(slot);
+		}
+	}
+	mutex_unlock(&slot_list_lock);
 }
 
 #ifdef CONFIG_DMI



  parent reply	other threads:[~2008-03-21  4:16 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-18 21:05 [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects Alex Chiang
2008-03-18 21:08 ` [PATCH 1/3] Construct one fakephp slot per pci slot Alex Chiang
2008-03-18 21:09 ` [PATCH 2/3] Introduce pci_slot Alex Chiang
2008-03-18 21:09 ` [PATCH 3/3] ACPI PCI slot detection driver Alex Chiang
2008-03-19  0:55 ` [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects Matthew Wilcox
2008-03-19  0:55   ` Matthew Wilcox
2008-03-19  1:52   ` Alex Chiang
2008-03-19  2:34     ` Kenji Kaneshige
2008-03-19  2:34       ` Kenji Kaneshige
2008-03-19  2:24   ` Kenji Kaneshige
2008-03-21  4:07 ` Kenji Kaneshige
2008-03-21  4:09   ` [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!) Kenji Kaneshige
2008-03-21 15:56     ` Alex Chiang
2008-03-21 16:15       ` Greg KH
2008-03-21 16:15         ` Greg KH
2008-03-21 16:45         ` Alex Chiang
2008-03-21  4:10   ` [PATCH 2/16] ACPI pci_slot: Fix dmi table for Fujitsu PRIMEQUEST " Kenji Kaneshige
2008-03-21 16:04     ` Alex Chiang
2008-03-21  4:11   ` [PATCH 3/16][BUG] ACPI pci_slot: Fix _STA evaluation " Kenji Kaneshige
2008-03-21 16:17     ` Alex Chiang
2008-03-21  4:12   ` [PATCH 4/16][BUG] PCI slot: Add missing semaphore for slot release " Kenji Kaneshige
2008-03-21 16:57     ` Alex Chiang
2008-03-21  4:13   ` [PATCH 5/16] PCI slot: Use list_head for pci slot list " Kenji Kaneshige
2008-03-21 18:40     ` Alex Chiang
2008-03-21  4:14   ` Kenji Kaneshige [this message]
2008-03-21 19:42     ` [PATCH 6/16][BUG] ACPI pci_slot: Fix slot removal path " Alex Chiang
2008-03-21  4:14   ` [PATCH 7/16][BUG] PCI slot: Remove compiler warnings " Kenji Kaneshige
2008-03-21 20:01     ` Alex Chiang
2008-03-21  4:15   ` [PATCH 8/16][BUG] PCI slot: Fix invalid memory access " Kenji Kaneshige
2008-03-21 20:01     ` Alex Chiang
2008-03-21  4:16   ` [PATCH 9/16] PCI slot: Remove unused slot member from pci_dev " Kenji Kaneshige
2008-03-21 19:30     ` Matthew Wilcox
2008-03-24 20:29       ` Alex Chiang
2008-03-21  4:17   ` [PATCH 10/16] PCI slot: Replace dbg with pr_debug " Kenji Kaneshige
2008-03-21 19:30     ` Matthew Wilcox
2008-03-21 20:02     ` Alex Chiang
2008-03-21  4:18   ` [PATCH 11/16] PCI slot: Remove useless release handler " Kenji Kaneshige
2008-03-25  3:08     ` Alex Chiang
2008-03-21  4:19   ` [PATCH 12/16] PCI slot: Use .default_attrs for address file " Kenji Kaneshige
2008-03-21 19:32     ` Matthew Wilcox
2008-03-25  3:31     ` Alex Chiang
2008-03-21  4:23   ` [PATCH 13/16] PCI slot: Fix return value of pci_create_slot() " Kenji Kaneshige
2008-03-25  3:31     ` Alex Chiang
2008-03-21  4:26   ` [PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() " Kenji Kaneshige
2008-03-21 19:32     ` Matthew Wilcox
2008-03-25  3:31     ` Alex Chiang
2008-03-21  4:26   ` [PATCH 15/16] PCI slot: Trivial cleanups for slot.c " Kenji Kaneshige
2008-03-21 19:33     ` Matthew Wilcox
2008-03-25  3:31     ` Alex Chiang
2008-03-21  4:27   ` [PATCH 16/16][BUG] PCI hotplug core: add missing lock for hotplug slot list " Kenji Kaneshige
2008-03-25  3:31     ` Alex Chiang
2008-03-21 15:53   ` [PATCH 0/3, v10] PCI, ACPI: Physical PCI slot objects Alex Chiang

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=47E3360B.1010506@jp.fujitsu.com \
    --to=kaneshige.kenji@jp.fujitsu.com \
    --cc=achiang@hp.com \
    --cc=garyhade@us.ibm.com \
    --cc=gregkh@suse.de \
    --cc=kristen.c.accardi@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@atrey.karlin.mff.cuni.cz \
    --cc=matthew@wil.cx \
    --cc=rick.jones2@hp.com \
    --cc=warthog19@eaglescrag.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.