Re: [GIT PULL] PCI changes for v3.9

[Yinghai Lu <yinghai@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1299 bytes --]

On Tue, Feb 26, 2013 at 8:13 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Feb 25, 2013 at 10:46 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Mon, Feb 25, 2013 at 9:19 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>>
>>> Also, my *gut* feel is that the new _handle_hotplug_event_root()
>>> function should do that whole dance with
>>> acpi_scan_lock_acquire()/acpi_scan_lock_release(), but I didn't really
>>> know if it's required or appropriate, so I left it alone. Could you
>>> take a look?
>>
>> Yes, we need that for root bridge hot add path.
>>
>> for hot remove path, we already have lock acquire/release in
>> acpi_bus_hot_remove_device().
>>
>> Please check attached patch for hot add path.
>
> Quite frankly, doing this in handle_root_bridge_insertion() doesn't
> match the pattern elsewhere. Elsewhere you also protected the whole
> acpi_get_name() lookup etc. Which is why I felt that it would make
> more sense to add this to _handle_hotplug_event_root().
>
> But there may be good reasons why the root bridge case is different,
> and I don't have strong opinions, I just wanted people to look at his
> case. I'll let you and Bjorn sort it out...

ok,

Bjorn, Rafael,

Can you please check if you are ok with attached patch ?

Thanks

Yinghai

[-- Attachment #2: fix_acpi_pci_root_acquire_lock.patch --]
[-- Type: application/octet-stream, Size: 1963 bytes --]

Subject: [PATCH] PCI, ACPI: hold acpi_scan_lock during root bus hotplug

During merging pci tree with pm/acpi tree, Linus noticed that
we don't have same lock using patten about acpi pci root as
acpiphp.

Here apply same lock patten, and we need to change
acpi_bus_hot_remove_device executing via acpi_os_hotplug_execute()
as it also hold the lock in acpi_bus_hot_remove_device.

That will make acpi_bus_hot_remove_device calling the same as other
callers.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/pci_root.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -652,6 +652,7 @@ static void handle_root_bridge_insertion
 
 static void handle_root_bridge_removal(struct acpi_device *device)
 {
+	acpi_status status;
 	struct acpi_eject_event *ej_event;
 
 	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
@@ -667,7 +668,9 @@ static void handle_root_bridge_removal(s
 	ej_event->device = device;
 	ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
 
-	acpi_bus_hot_remove_device(ej_event);
+	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
+	if (ACPI_FAILURE(status))
+		kfree(ej_event);
 }
 
 static void _handle_hotplug_event_root(struct work_struct *work)
@@ -682,8 +685,9 @@ static void _handle_hotplug_event_root(s
 	handle = hp_work->handle;
 	type = hp_work->type;
 
-	root = acpi_pci_find_root(handle);
+	acpi_scan_lock_acquire();
 
+	root = acpi_pci_find_root(handle);
 	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 
 	switch (type) {
@@ -717,6 +721,7 @@ static void _handle_hotplug_event_root(s
 		break;
 	}
 
+	acpi_scan_lock_release();
 	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 	kfree(buffer.pointer);
 }