public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
To: Alex Chiang <achiang@hp.com>
Cc: jbarnes@virtuousgeek.org, kristen.c.accardi@intel.com,
	matthew@wil.cx, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [02/03] Sample patch for [PATCH v4 03/15]
Date: Wed, 08 Oct 2008 15:34:51 +0900	[thread overview]
Message-ID: <48EC548B.5020704@jp.fujitsu.com> (raw)
In-Reply-To: <48EC53D3.9020203@jp.fujitsu.com>

NOTE:This doesn't target the comment about changing exported
symbol name.

---
 drivers/pci/hotplug/pci_hotplug_core.c |   18 +---
 drivers/pci/hotplug/pciehp_core.c      |   15 ----
 drivers/pci/hotplug/shpchp_core.c      |   15 ----
 drivers/pci/slot.c                     |  123 ++++++++++++++++++++++++++-------
 include/linux/pci.h                    |    4 -
 5 files changed, 107 insertions(+), 68 deletions(-)

Index: hotplug/drivers/pci/hotplug/pciehp_core.c
===================================================================
--- hotplug.orig/drivers/pci/hotplug/pciehp_core.c
+++ hotplug/drivers/pci/hotplug/pciehp_core.c
@@ -196,7 +196,6 @@ static int init_slots(struct controller 
 	struct slot *slot;
 	struct hotplug_slot *hotplug_slot;
 	struct hotplug_slot_info *info;
-	int len, dup = 1;
 	int retval = -ENOMEM;
 
 	list_for_each_entry(slot, &ctrl->slot_list, slot_list) {
@@ -223,25 +222,11 @@ static int init_slots(struct controller 
 		ctrl_dbg(ctrl, "Registering bus=%x dev=%x hp_slot=%x sun=%x "
 			 "slot_device_offset=%x\n", slot->bus, slot->device,
 			 slot->hp_slot, slot->number, ctrl->slot_device_offset);
-duplicate_name:
 		retval = pci_hp_register(hotplug_slot,
 					 ctrl->pci_dev->subordinate,
 					 slot->device,
 					 slot->name);
 		if (retval) {
-			/*
-			 * If slot N already exists, we'll try to create
-			 * slot N-1, N-2 ... N-M, until we overflow.
-			 */
-			if (retval == -EEXIST) {
-				len = snprintf(slot->name, SLOT_NAME_SIZE,
-					       "%d-%d", slot->number, dup++);
-				if (len < SLOT_NAME_SIZE)
-					goto duplicate_name;
-				else
-					ctrl_err(ctrl, "duplicate slot name "
-						 "overflow\n");
-			}
 			ctrl_err(ctrl, "pci_hp_register failed with error %d\n",
 				 retval);
 			goto error_info;
Index: hotplug/drivers/pci/hotplug/shpchp_core.c
===================================================================
--- hotplug.orig/drivers/pci/hotplug/shpchp_core.c
+++ hotplug/drivers/pci/hotplug/shpchp_core.c
@@ -102,7 +102,7 @@ static int init_slots(struct controller 
 	struct hotplug_slot *hotplug_slot;
 	struct hotplug_slot_info *info;
 	int retval = -ENOMEM;
-	int i, len, dup = 1;
+	int i;
 
 	for (i = 0; i < ctrl->num_slots; i++) {
 		slot = kzalloc(sizeof(*slot), GFP_KERNEL);
@@ -144,23 +144,10 @@ static int init_slots(struct controller 
 		dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
 		    "slot_device_offset=%x\n", slot->bus, slot->device,
 		    slot->hp_slot, slot->number, ctrl->slot_device_offset);
-duplicate_name:
 		retval = pci_hp_register(slot->hotplug_slot,
 				ctrl->pci_dev->subordinate, slot->device,
 				hotplug_slot->name);
 		if (retval) {
-			/*
-			 * If slot N already exists, we'll try to create
-			 * slot N-1, N-2 ... N-M, until we overflow.
-			 */
-			if (retval == -EEXIST) {
-				len = snprintf(slot->name, SLOT_NAME_SIZE,
-					       "%d-%d", slot->number, dup++);
-				if (len < SLOT_NAME_SIZE)
-					goto duplicate_name;
-				else
-					err("duplicate slot name overflow\n");
-			}
 			err("pci_hp_register failed with error %d\n", retval);
 			goto error_info;
 		}
Index: hotplug/include/linux/pci.h
===================================================================
--- hotplug.orig/include/linux/pci.h
+++ hotplug/include/linux/pci.h
@@ -509,8 +509,10 @@ struct pci_bus *pci_add_new_bus(struct p
 				int busnr);
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 				 const char *name);
+struct pci_slot *pci_get_pci_slot(struct pci_bus *parent, int slot_nr);
 void pci_destroy_slot(struct pci_slot *slot);
-void pci_update_slot_number(struct pci_slot *slot, int slot_nr);
+int pci_rename_slot(struct pci_slot *slot, const char *name);
+void pci_renumber_slot(struct pci_slot *slot, int slot_nr);
 int pci_scan_slot(struct pci_bus *bus, int devfn);
 struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn);
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus);
Index: hotplug/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================
--- hotplug.orig/drivers/pci/hotplug/pci_hotplug_core.c
+++ hotplug/drivers/pci/hotplug/pci_hotplug_core.c
@@ -569,12 +569,6 @@ int pci_hp_register(struct hotplug_slot 
 
 	mutex_lock(&pci_hp_mutex);
 
-	/* Check if we have already registered a slot with the same name. */
-	if (get_slot_from_name(name)) {
-		result = -EEXIST;
-		goto out;
-	}
-
 	/*
 	 * No problems if we call this interface from both ACPI_PCI_SLOT
 	 * driver and call it here again. If we've already created the
@@ -590,18 +584,14 @@ int pci_hp_register(struct hotplug_slot 
 		goto cleanup;
 	}
 
-	slot->pci_slot = pci_slot;
-	pci_slot->hotplug = slot;
-
 	/*
 	 * Allow pcihp drivers to override the ACPI_PCI_SLOT name.
 	 */
-	if (strcmp(kobject_name(&pci_slot->kobj), name)) {
-		result = kobject_rename(&pci_slot->kobj, name);
-		if (result)
-			goto cleanup;
-	}
+	if ((result = pci_rename_slot(pci_slot, name)))
+		goto cleanup;
 
+	slot->pci_slot = pci_slot;
+	pci_slot->hotplug = slot;
 	list_add(&slot->slot_list, &pci_hotplug_slot_list);
 
 	result = fs_add_slot(pci_slot);
Index: hotplug/drivers/pci/slot.c
===================================================================
--- hotplug.orig/drivers/pci/slot.c
+++ hotplug/drivers/pci/slot.c
@@ -78,6 +78,46 @@ static struct kobj_type pci_slot_ktype =
 	.default_attrs = pci_slot_default_attrs,
 };
 
+static char *make_slot_name(struct pci_slot *slot, const char *name)
+{
+	char *new_name;
+	int len, max, dup;
+
+	new_name = kstrdup(name, GFP_KERNEL);
+	if (!new_name)
+		return NULL;
+
+	/*
+	 * Make sure we hit the realloc case the first time through the
+	 * loop.  'len' will be strlen(name) + 3 at that point which is
+	 * enough space for "name-X" and the trailing NUL.
+	 */
+	len = strlen(name) + 2;
+	max = 1;
+	dup = 1;
+
+	for (;;) {
+		struct kobject *dup_slot;
+		dup_slot = kset_find_obj(pci_slots_kset, new_name);
+		if (!dup_slot)
+			break;
+		kobject_put(dup_slot);
+		if (dup_slot == &slot->kobj)
+			break;
+		if (dup == max) {
+			len++;
+			max *= 10;
+			kfree(new_name);
+			new_name = kmalloc(len, GFP_KERNEL);
+			if (!new_name)
+				break;
+		}
+		sprintf(new_name, "%s-%d", name, dup++);
+	}
+
+	return new_name;
+}
+
 /**
  * pci_create_slot - create or increment refcount for physical PCI slot
  * @parent: struct pci_bus of parent bridge
@@ -89,7 +129,19 @@ static struct kobj_type pci_slot_ktype =
  * either return a new &struct pci_slot to the caller, or if the pci_slot
  * already exists, its refcount will be incremented.
  *
- * Slots are uniquely identified by a @pci_bus, @slot_nr, @name tuple.
+ * Slots are uniquely identified by a @pci_bus, @slot_nr tuple.
+ *
+ * The kobject API imposes a restriction on us, and does not allow sysfs
+ * entries with duplicate names. There are known platforms with broken
+ * firmware that assign the same name to multiple slots.
+ *
+ * We workaround these broken platforms by renaming the slots on behalf
+ * of the caller. If firmware assigns name N to multiple slots:
+ *
+ * The first slot is assigned N
+ * The second slot is assigned N-1
+ * The third slot is assigned N-2
+ * etc.
  *
  * Placeholder slots:
  * In most cases, @pci_bus, @slot_nr will be sufficient to uniquely identify
@@ -98,24 +150,20 @@ static struct kobj_type pci_slot_ktype =
  * the slot. In this scenario, the caller may pass -1 for @slot_nr.
  *
  * The following semantics are imposed when the caller passes @slot_nr ==
- * -1. First, the check for existing %struct pci_slot is skipped, as the
- * caller may know about several unpopulated slots on a given %struct
- * pci_bus, and each slot would have a @slot_nr of -1.  Uniqueness for
- * these slots is then determined by the @name parameter. We expect
- * kobject_init_and_add() to warn us if the caller attempts to create
- * multiple slots with the same name. The other change in semantics is
+ * -1. First, we no longer check for an existing %struct pci_slot, as there
+ * may be many slots with @slot_nr of -1.  The other change in semantics is
  * user-visible, which is the 'address' parameter presented in sysfs will
  * consist solely of a dddd:bb tuple, where dddd is the PCI domain of the
  * %struct pci_bus and bb is the bus number. In other words, the devfn of
  * the 'placeholder' slot will not be displayed.
  */
-
 struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
 				 const char *name)
 {
 	struct pci_dev *dev;
 	struct pci_slot *slot;
 	int err;
+	char *slot_name = NULL;
 
 	down_write(&pci_bus_sem);
 
@@ -144,18 +192,23 @@ placeholder:
 
 	slot->bus = parent;
 	slot->number = slot_nr;
-
 	slot->kobj.kset = pci_slots_kset;
+
+	slot_name = make_slot_name(slot, name);
+	if (!slot_name) {
+		err = -ENOMEM;
+		goto err;
+	}
+
 	err = kobject_init_and_add(&slot->kobj, &pci_slot_ktype, NULL,
-				   "%s", name);
+				   "%s", slot_name);
 	if (err) {
-		printk(KERN_ERR "Unable to register kobject %s\n", name);
+		printk(KERN_ERR "Unable to register kobject %s\n", slot_name);
 		goto err;
 	}
 
 	INIT_LIST_HEAD(&slot->list);
 	list_add(&slot->list, &parent->slots);
-
 	list_for_each_entry(dev, &parent->devices, bus_list)
 		if (PCI_SLOT(dev->devfn) == slot_nr)
 			dev->slot = slot;
@@ -166,6 +219,7 @@ placeholder:
 
  out:
 	up_write(&pci_bus_sem);
+	kfree(slot_name);
 	return slot;
  err:
 	kfree(slot);
@@ -175,7 +229,35 @@ placeholder:
 EXPORT_SYMBOL_GPL(pci_create_slot);
 
 /**
- * pci_update_slot_number - update %struct pci_slot -> number
+ * pci_rename_slot - update %struct pci_slot -> name
+ * @slot - %struct pci_slot to update
+ * @name - new requested name for slot
+ *
+ * Allow caller to safely rename a %struct pci_slot without making
+ * the kobject core complain about duplicate names.
+ */
+int pci_rename_slot(struct pci_slot *slot, const char *name)
+{
+	int result = 0;
+	char *slot_name;
+
+	down_write(&pci_bus_sem);
+	slot_name = make_slot_name(slot, name);
+	if (!slot_name) {
+		result = -ENOMEM;
+		goto out;
+	}
+	if (strcmp(kobject_name(&slot->kobj), slot_name))
+		result = kobject_rename(&slot->kobj, slot_name);
+	kfree(slot_name);
+out:
+	up_write(&pci_bus_sem);
+	return result;
+}
+EXPORT_SYMBOL_GPL(pci_rename_slot);
+
+/**
+ * pci_renumber_slot - update %struct pci_slot -> number
  * @slot - %struct pci_slot to update
  * @slot_nr - new number for slot
  *
@@ -183,27 +265,21 @@ EXPORT_SYMBOL_GPL(pci_create_slot);
  * created a placeholder slot in pci_create_slot() by passing a -1 as
  * slot_nr, to update their %struct pci_slot with the correct @slot_nr.
  */
-
-void pci_update_slot_number(struct pci_slot *slot, int slot_nr)
+void pci_renumber_slot(struct pci_slot *slot, int slot_nr)
 {
-	int name_count = 0;
 	struct pci_slot *tmp;
 
 	down_write(&pci_bus_sem);
 
 	list_for_each_entry(tmp, &slot->bus->slots, list) {
 		WARN_ON(tmp->number == slot_nr);
-		if (!strcmp(kobject_name(&tmp->kobj), kobject_name(&slot->kobj)))
-			name_count++;
+		goto out;
 	}
-
-	if (name_count > 1)
-		printk(KERN_WARNING "pci_update_slot_number found %d slots with the same name: %s\n", name_count, kobject_name(&slot->kobj));
-
 	slot->number = slot_nr;
+out:
 	up_write(&pci_bus_sem);
 }
-EXPORT_SYMBOL_GPL(pci_update_slot_number);
+EXPORT_SYMBOL_GPL(pci_renumber_slot);
 
 /**
  * pci_destroy_slot - decrement refcount for physical PCI slot
@@ -213,7 +289,6 @@ EXPORT_SYMBOL_GPL(pci_update_slot_number
  * just call kobject_put on its kobj and let our release methods do the
  * rest.
  */
-
 void pci_destroy_slot(struct pci_slot *slot)
 {
 	pr_debug("%s: dec refcount to %d on %04x:%02x:%02x\n", __func__,



  parent reply	other threads:[~2008-10-08  6:37 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-03 23:17 [PATCH v4 00/15] PCI: let the core manage slot names Alex Chiang
2008-10-03 23:17 ` [PATCH v4 01/15] PCI Hotplug core: add 'name' param pci_hp_register interface Alex Chiang
2008-10-06  9:04   ` Matthew Wilcox
2008-10-03 23:17 ` [PATCH v4 02/15] PCI Hotplug: serialize pci_hp_register/deregister Alex Chiang
2008-10-06 14:45   ` Matthew Wilcox
2008-10-09  4:09     ` Alex Chiang
2008-10-08  5:42   ` Kenji Kaneshige
2008-10-03 23:17 ` [PATCH v4 03/15] PCI: prevent duplicate slot names Alex Chiang
2008-10-08  6:00   ` Kenji Kaneshige
2008-10-09  4:12     ` Alex Chiang
2008-10-03 23:17 ` [PATCH v4 04/15] PCI, PCI Hotplug: introduce slot_name helpers Alex Chiang
2008-10-03 23:17 ` [PATCH v4 05/15] PCI: acpiphp: remove 'name' parameter Alex Chiang
2008-10-03 23:17 ` [PATCH v4 06/15] PCI: cpci_hotplug: stop managing hotplug_slot->name Alex Chiang
2008-10-03 23:18 ` [PATCH v4 07/15] PCI: cpqphp: " Alex Chiang
2008-10-03 23:18 ` [PATCH v4 08/15] PCI: fakephp: remove 'name' parameter Alex Chiang
2008-10-03 23:18 ` [PATCH v4 09/15] PCI: ibmphp: stop managing hotplug_slot->name Alex Chiang
2008-10-03 23:18 ` [PATCH v4 10/15] PCI: pciehp: remove 'name' parameter Alex Chiang
2008-10-03 23:18 ` [PATCH v4 11/15] PCI: rpaphp: kmalloc/kfree slot->name directly Alex Chiang
2008-10-06  6:44   ` Pekka Enberg
2008-10-09  4:05     ` Alex Chiang
2008-10-03 23:18 ` [PATCH v4 12/15] PCI: SGI Hotplug: stop managing bss_hotplug_slot->name Alex Chiang
2008-10-03 23:18 ` [PATCH v4 13/15] PCI: shcphp: remove 'name' parameter Alex Chiang
2008-10-03 23:18 ` [PATCH v4 14/15] PCI: Hotplug core: remove 'name' Alex Chiang
2008-10-03 23:18 ` [PATCH v4 15/15] PCI Hotplug: fakephp: add duplicate slot name debugging Alex Chiang
2008-10-08  6:31 ` [PATCH v4 00/15] PCI: let the core manage slot names Kenji Kaneshige
2008-10-08  6:33   ` [01/03] Sample patch for [PATCH v4 02/15] Kenji Kaneshige
2008-10-08  6:34   ` Kenji Kaneshige [this message]
2008-10-08  6:36   ` [03/03] Sample patch for [PATCH v4 14/15] Kenji Kaneshige
2008-10-09  4:19   ` [PATCH v4 00/15] PCI: let the core manage slot names Alex Chiang
2008-10-09  5:01     ` Kenji Kaneshige

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=48EC548B.5020704@jp.fujitsu.com \
    --to=kaneshige.kenji@jp.fujitsu.com \
    --cc=achiang@hp.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=kristen.c.accardi@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=matthew@wil.cx \
    /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