diff mbox

PCI, ACPI: hold acpi_scan_lock during root bus hotplug

Message ID 1362978316-23553-1-git-send-email-yinghai@kernel.org
State Not Applicable
Headers show

Commit Message

Yinghai Lu March 11, 2013, 5:05 a.m. UTC
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>
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Rafael J. Wysocki March 26, 2013, 1:24 p.m. UTC | #1
On Sunday, March 10, 2013 10:05:16 PM Yinghai Lu wrote:
> 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>
> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

Bjorn, this needs to go in before final 3.9.

Are you going to handle it, or should I take care of it?

Rafael


> ---
>  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
> @@ -646,6 +646,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);
> @@ -661,7 +662,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)
> @@ -676,8 +679,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) {
> @@ -711,6 +715,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);
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu March 26, 2013, 3:12 p.m. UTC | #2
On Tue, Mar 26, 2013 at 6:24 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Sunday, March 10, 2013 10:05:16 PM Yinghai Lu wrote:
>> 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>
>> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
>
> Bjorn, this needs to go in before final 3.9.
>
> Are you going to handle it, or should I take care of it?

It should be better via your pm+acpi tree, as that lock is added via your tree.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas March 26, 2013, 10:48 p.m. UTC | #3
On Tue, Mar 26, 2013 at 4:54 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, March 26, 2013 08:12:30 AM Yinghai Lu wrote:
>> On Tue, Mar 26, 2013 at 6:24 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Sunday, March 10, 2013 10:05:16 PM Yinghai Lu wrote:
>> >> 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>
>> >> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
>> >
>> > Bjorn, this needs to go in before final 3.9.
>> >
>> > Are you going to handle it, or should I take care of it?
>>
>> It should be better via your pm+acpi tree, as that lock is added via your tree.
>
> OK, I'll take it, then, if Bjorn doesn't object.

Yep, that's fine with me.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki March 26, 2013, 10:54 p.m. UTC | #4
On Tuesday, March 26, 2013 08:12:30 AM Yinghai Lu wrote:
> On Tue, Mar 26, 2013 at 6:24 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Sunday, March 10, 2013 10:05:16 PM Yinghai Lu wrote:
> >> 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>
> >> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Bjorn, this needs to go in before final 3.9.
> >
> > Are you going to handle it, or should I take care of it?
> 
> It should be better via your pm+acpi tree, as that lock is added via your tree.

OK, I'll take it, then, if Bjorn doesn't object.

Thanks,
Rafael
Rafael J. Wysocki March 26, 2013, 10:58 p.m. UTC | #5
On Tuesday, March 26, 2013 04:48:49 PM Bjorn Helgaas wrote:
> On Tue, Mar 26, 2013 at 4:54 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Tuesday, March 26, 2013 08:12:30 AM Yinghai Lu wrote:
> >> On Tue, Mar 26, 2013 at 6:24 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Sunday, March 10, 2013 10:05:16 PM Yinghai Lu wrote:
> >> >> 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>
> >> >> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
> >> >
> >> > Bjorn, this needs to go in before final 3.9.
> >> >
> >> > Are you going to handle it, or should I take care of it?
> >>
> >> It should be better via your pm+acpi tree, as that lock is added via your tree.
> >
> > OK, I'll take it, then, if Bjorn doesn't object.
> 
> Yep, that's fine with me.

Cool, thanks. :-)
diff mbox

Patch

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
@@ -646,6 +646,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);
@@ -661,7 +662,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)
@@ -676,8 +679,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) {
@@ -711,6 +715,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);
 }