| Message ID | 3032117.IcK9I7rYQT@vostro.rjw.lan (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sat, Jan 26, 2013 at 2:41 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > There is no guarantee that acpi_bus_scan() and acpi_bus_trim() will > not be run in parallel for the same scope of the ACPI namespace, > which may lead to a great deal of confusion, so introduce a new mutex > to prevent that from happening. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by: Yinghai Lu <yinghai@kernel.org> Steven, Can you apply this one to for-pci-res-alloc to check if racing with docking hotplug/eject still happen? or wait one or two days after i rebase that branch. Thanks Yinghai > --- > drivers/acpi/scan.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -52,6 +52,7 @@ static const struct acpi_device_id acpi_ > > static LIST_HEAD(acpi_device_list); > static LIST_HEAD(acpi_bus_id_list); > +static DEFINE_MUTEX(acpi_scan_lock); > DEFINE_MUTEX(acpi_device_lock); > LIST_HEAD(acpi_wakeup_device_list); > > @@ -1612,19 +1613,22 @@ static acpi_status acpi_bus_device_attac > int acpi_bus_scan(acpi_handle handle) > { > void *device = NULL; > + int error = 0; > + > + mutex_lock(&acpi_scan_lock); > > if (ACPI_SUCCESS(acpi_bus_check_add(handle, 0, NULL, &device))) > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > acpi_bus_check_add, NULL, NULL, &device); > > if (!device) > - return -ENODEV; > - > - if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) > + error = -ENODEV; > + else if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > acpi_bus_device_attach, NULL, NULL, NULL); > > - return 0; > + mutex_unlock(&acpi_scan_lock); > + return error; > } > EXPORT_SYMBOL(acpi_bus_scan); > > @@ -1653,6 +1657,8 @@ static acpi_status acpi_bus_remove(acpi_ > > void acpi_bus_trim(struct acpi_device *start) > { > + mutex_lock(&acpi_scan_lock); > + > /* > * Execute acpi_bus_device_detach() as a post-order callback to detach > * all ACPI drivers from the device nodes being removed. > @@ -1667,6 +1673,8 @@ void acpi_bus_trim(struct acpi_device *s > acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL, > acpi_bus_remove, NULL, NULL); > acpi_bus_remove(start->handle, 0, NULL, NULL); > + > + mutex_unlock(&acpi_scan_lock); > } > EXPORT_SYMBOL_GPL(acpi_bus_trim); > > -- 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
On Sat, 2013-01-26 at 15:19 -0800, Yinghai Lu wrote: > On Sat, Jan 26, 2013 at 2:41 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > There is no guarantee that acpi_bus_scan() and acpi_bus_trim() will > > not be run in parallel for the same scope of the ACPI namespace, > > which may lead to a great deal of confusion, so introduce a new mutex > > to prevent that from happening. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Acked-by: Yinghai Lu <yinghai@kernel.org> > > Steven, > > Can you apply this one to for-pci-res-alloc to check if racing with > docking hotplug/eject > still happen? > or wait one or two days after i rebase that branch. > Still get the corrupt acpi_handle. > Thanks > > Yinghai > > > --- > > drivers/acpi/scan.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > Index: linux-pm/drivers/acpi/scan.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/scan.c > > +++ linux-pm/drivers/acpi/scan.c > > @@ -52,6 +52,7 @@ static const struct acpi_device_id acpi_ > > > > static LIST_HEAD(acpi_device_list); > > static LIST_HEAD(acpi_bus_id_list); > > +static DEFINE_MUTEX(acpi_scan_lock); > > DEFINE_MUTEX(acpi_device_lock); > > LIST_HEAD(acpi_wakeup_device_list); > > > > @@ -1612,19 +1613,22 @@ static acpi_status acpi_bus_device_attac > > int acpi_bus_scan(acpi_handle handle) > > { > > void *device = NULL; > > + int error = 0; > > + > > + mutex_lock(&acpi_scan_lock); > > > > if (ACPI_SUCCESS(acpi_bus_check_add(handle, 0, NULL, &device))) > > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > > acpi_bus_check_add, NULL, NULL, &device); > > > > if (!device) > > - return -ENODEV; > > - > > - if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) > > + error = -ENODEV; > > + else if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) > > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > > acpi_bus_device_attach, NULL, NULL, NULL); > > > > - return 0; > > + mutex_unlock(&acpi_scan_lock); > > + return error; > > } > > EXPORT_SYMBOL(acpi_bus_scan); > > > > @@ -1653,6 +1657,8 @@ static acpi_status acpi_bus_remove(acpi_ > > > > void acpi_bus_trim(struct acpi_device *start) > > { > > + mutex_lock(&acpi_scan_lock); > > + > > /* > > * Execute acpi_bus_device_detach() as a post-order callback to detach > > * all ACPI drivers from the device nodes being removed. > > @@ -1667,6 +1673,8 @@ void acpi_bus_trim(struct acpi_device *s > > acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL, > > acpi_bus_remove, NULL, NULL); > > acpi_bus_remove(start->handle, 0, NULL, NULL); > > + > > + mutex_unlock(&acpi_scan_lock); > > } > > EXPORT_SYMBOL_GPL(acpi_bus_trim); > > > > -- 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
On Sat, 2013-01-26 at 15:19 -0800, Yinghai Lu wrote: > On Sat, Jan 26, 2013 at 2:41 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > There is no guarantee that acpi_bus_scan() and acpi_bus_trim() will > > not be run in parallel for the same scope of the ACPI namespace, > > which may lead to a great deal of confusion, so introduce a new mutex > > to prevent that from happening. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Acked-by: Yinghai Lu <yinghai@kernel.org> > > Steven, > > Can you apply this one to for-pci-res-alloc to check if racing with > docking hotplug/eject > still happen? > or wait one or two days after i rebase that branch. Tried merging with linux-pm/bleeding-edge, same behaviour: [ 3589.013578] ACPI: \_SB_.PCI0.PCIE.GDCK: undocking [ 3589.585356] vgaarb: device changed decodes: PCI:0000:00:02.0,olddecodes=none,decodes=io+mem:owns=none [ 3589.585422] ACPI: Delete PCI Interrupt Routing Table for 0000:04 [ 3589.585426] pci 0000:03:08.0: Oops, 'acpi_handle' corrupt [ 3589.585446] pci_bus 0000:04: busn_res: [bus 04] is released 03:08.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI Bridge (rev aa) 04:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Manhattan [Mobility Radeon HD 5430 Series] 04:00.1 Audio device: Advanced Micro Devices [AMD] nee ATI Cedar HDMI Audio [Radeon HD 5400/6300 Series] > > --- > > drivers/acpi/scan.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > Index: linux-pm/drivers/acpi/scan.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/scan.c > > +++ linux-pm/drivers/acpi/scan.c > > @@ -52,6 +52,7 @@ static const struct acpi_device_id acpi_ > > > > static LIST_HEAD(acpi_device_list); > > static LIST_HEAD(acpi_bus_id_list); > > +static DEFINE_MUTEX(acpi_scan_lock); > > DEFINE_MUTEX(acpi_device_lock); > > LIST_HEAD(acpi_wakeup_device_list); > > > > @@ -1612,19 +1613,22 @@ static acpi_status acpi_bus_device_attac > > int acpi_bus_scan(acpi_handle handle) > > { > > void *device = NULL; > > + int error = 0; > > + > > + mutex_lock(&acpi_scan_lock); > > > > if (ACPI_SUCCESS(acpi_bus_check_add(handle, 0, NULL, &device))) > > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > > acpi_bus_check_add, NULL, NULL, &device); > > > > if (!device) > > - return -ENODEV; > > - > > - if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) > > + error = -ENODEV; > > + else if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) > > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > > acpi_bus_device_attach, NULL, NULL, NULL); > > > > - return 0; > > + mutex_unlock(&acpi_scan_lock); > > + return error; > > } > > EXPORT_SYMBOL(acpi_bus_scan); > > > > @@ -1653,6 +1657,8 @@ static acpi_status acpi_bus_remove(acpi_ > > > > void acpi_bus_trim(struct acpi_device *start) > > { > > + mutex_lock(&acpi_scan_lock); > > + > > /* > > * Execute acpi_bus_device_detach() as a post-order callback to detach > > * all ACPI drivers from the device nodes being removed. > > @@ -1667,6 +1673,8 @@ void acpi_bus_trim(struct acpi_device *s > > acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL, > > acpi_bus_remove, NULL, NULL); > > acpi_bus_remove(start->handle, 0, NULL, NULL); > > + > > + mutex_unlock(&acpi_scan_lock); > > } > > EXPORT_SYMBOL_GPL(acpi_bus_trim); > > > > -- 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
On Saturday, February 02, 2013 11:58:41 AM Steven Newbury wrote: > On Sat, 2013-01-26 at 15:19 -0800, Yinghai Lu wrote: > > On Sat, Jan 26, 2013 at 2:41 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > There is no guarantee that acpi_bus_scan() and acpi_bus_trim() will > > > not be run in parallel for the same scope of the ACPI namespace, > > > which may lead to a great deal of confusion, so introduce a new mutex > > > to prevent that from happening. > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Acked-by: Yinghai Lu <yinghai@kernel.org> > > > > Steven, > > > > Can you apply this one to for-pci-res-alloc to check if racing with > > docking hotplug/eject > > still happen? > > or wait one or two days after i rebase that branch. > > Tried merging with linux-pm/bleeding-edge, same behaviour: > > [ 3589.013578] ACPI: \_SB_.PCI0.PCIE.GDCK: undocking > [ 3589.585356] vgaarb: device changed decodes: PCI:0000:00:02.0,olddecodes=none,decodes=io+mem:owns=none > [ 3589.585422] ACPI: Delete PCI Interrupt Routing Table for 0000:04 > [ 3589.585426] pci 0000:03:08.0: Oops, 'acpi_handle' corrupt > [ 3589.585446] pci_bus 0000:04: busn_res: [bus 04] is released > > 03:08.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI > Express-to-PCI Bridge (rev aa) > 04:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI > Manhattan [Mobility Radeon HD 5430 Series] > 04:00.1 Audio device: Advanced Micro Devices [AMD] nee ATI Cedar HDMI > Audio [Radeon HD 5400/6300 Series] That's because of a bug in the dock code I believe. If you're willing to test patches, I can try to cook up something to debug/fix this. Thanks, Rafael
On Sat, 2 Feb 2013, 20:18:28 GMT, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Saturday, February 02, 2013 11:58:41 AM Steven Newbury wrote: > > On Sat, 2013-01-26 at 15:19 -0800, Yinghai Lu wrote: > > > On Sat, Jan 26, 2013 at 2:41 PM, Rafael J. Wysocki <rjw@sisk.pl> > > > wrote: > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > There is no guarantee that acpi_bus_scan() and acpi_bus_trim() will > > > > not be run in parallel for the same scope of the ACPI namespace, > > > > which may lead to a great deal of confusion, so introduce a new > > > > mutex to prevent that from happening. > > > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > Acked-by: Yinghai Lu <yinghai@kernel.org> > > > > > > Steven, > > > > > > Can you apply this one to for-pci-res-alloc to check if racing with > > > docking hotplug/eject > > > still happen? > > > or wait one or two days after i rebase that branch. > > > > Tried merging with linux-pm/bleeding-edge, same behaviour: > > > > [ 3589.013578] ACPI: \_SB_.PCI0.PCIE.GDCK: undocking > > [ 3589.585356] vgaarb: device changed decodes: > > PCI:0000:00:02.0,olddecodes=none,decodes=io+mem:owns=none [ > > 3589.585422] ACPI: Delete PCI Interrupt Routing Table for 0000:04 [ > > 3589.585426] pci 0000:03:08.0: Oops, 'acpi_handle' corrupt [ > > 3589.585446] pci_bus 0000:04: busn_res: [bus 04] is released > > > > 03:08.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI > > Express-to-PCI Bridge (rev aa) > > 04:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI > > Manhattan [Mobility Radeon HD 5430 Series] > > 04:00.1 Audio device: Advanced Micro Devices [AMD] nee ATI Cedar HDMI > > Audio [Radeon HD 5400/6300 Series] > > That's because of a bug in the dock code I believe. > > If you're willing to test patches, I can try to cook up something to > debug/fix this. Sure, I'm always willing to test patches. -- 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
On Saturday, February 02, 2013 08:28:01 PM Steven Newbury wrote: > On Sat, 2 Feb 2013, 20:18:28 GMT, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > > On Saturday, February 02, 2013 11:58:41 AM Steven Newbury wrote: > > > On Sat, 2013-01-26 at 15:19 -0800, Yinghai Lu wrote: > > > > On Sat, Jan 26, 2013 at 2:41 PM, Rafael J. Wysocki <rjw@sisk.pl> > > > > wrote: > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > > > There is no guarantee that acpi_bus_scan() and acpi_bus_trim() will > > > > > not be run in parallel for the same scope of the ACPI namespace, > > > > > which may lead to a great deal of confusion, so introduce a new > > > > > mutex to prevent that from happening. > > > > > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > Acked-by: Yinghai Lu <yinghai@kernel.org> > > > > > > > > Steven, > > > > > > > > Can you apply this one to for-pci-res-alloc to check if racing with > > > > docking hotplug/eject > > > > still happen? > > > > or wait one or two days after i rebase that branch. > > > > > > Tried merging with linux-pm/bleeding-edge, same behaviour: > > > > > > [ 3589.013578] ACPI: \_SB_.PCI0.PCIE.GDCK: undocking > > > [ 3589.585356] vgaarb: device changed decodes: > > > PCI:0000:00:02.0,olddecodes=none,decodes=io+mem:owns=none [ > > > 3589.585422] ACPI: Delete PCI Interrupt Routing Table for 0000:04 [ > > > 3589.585426] pci 0000:03:08.0: Oops, 'acpi_handle' corrupt [ > > > 3589.585446] pci_bus 0000:04: busn_res: [bus 04] is released > > > > > > 03:08.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI > > > Express-to-PCI Bridge (rev aa) > > > 04:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI > > > Manhattan [Mobility Radeon HD 5430 Series] > > > 04:00.1 Audio device: Advanced Micro Devices [AMD] nee ATI Cedar HDMI > > > Audio [Radeon HD 5400/6300 Series] > > > > That's because of a bug in the dock code I believe. > > > > If you're willing to test patches, I can try to cook up something to > > debug/fix this. > Sure, I'm always willing to test patches. Can you please test this one in addition to the $subject one: https://patchwork.kernel.org/patch/2068621/ and see if that helps? Rafael
On Sat, 2013-02-02 at 23:27 +0100, Rafael J. Wysocki wrote: > On Saturday, February 02, 2013 08:28:01 PM Steven Newbury wrote: > > > > Tried merging with linux-pm/bleeding-edge, same behaviour: > > > > > > > > [ 3589.013578] ACPI: \_SB_.PCI0.PCIE.GDCK: undocking > > > > [ 3589.585356] vgaarb: device changed decodes: > > > > PCI:0000:00:02.0,olddecodes=none,decodes=io+mem:owns=none [ > > > > 3589.585422] ACPI: Delete PCI Interrupt Routing Table for 0000:04 [ > > > > 3589.585426] pci 0000:03:08.0: Oops, 'acpi_handle' corrupt [ > > > > 3589.585446] pci_bus 0000:04: busn_res: [bus 04] is released > > > > > > > > 03:08.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI > > > > Express-to-PCI Bridge (rev aa) > > > > 04:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI > > > > Manhattan [Mobility Radeon HD 5430 Series] > > > > 04:00.1 Audio device: Advanced Micro Devices [AMD] nee ATI Cedar HDMI > > > > Audio [Radeon HD 5400/6300 Series] > > > > > > That's because of a bug in the dock code I believe. > > > > > > If you're willing to test patches, I can try to cook up something to > > > debug/fix this. > > Sure, I'm always willing to test patches. > > Can you please test this one in addition to the $subject one: > > https://patchwork.kernel.org/patch/2068621/ > > and see if that helps? No change wrt the "Oops, 'acpi_handle' corrupt". Just to be clear, this happens during the logical undock phase after writing to "/sys/devices/platform/dock.0/undock". Right now I manually tear down the seat associated with the dock gfx card (waiting for the X server to terminate and requiring SIGSTOPing the gdm process to stop it respawning!) then attempt radeon module unload, if it succeeds I write to the undock file, otherwise I fail the undock; I saw there is intention to put it infrastructure to have drivers prepare themselves for removal on 'eject' request, that would be really helpful. A couple of other things regarding the dock system: Making an undock/eject request with the dock eject button results in an ACPI 'undock' event, same ACPI event happens when the electrical/physical undock occurs. The IDE docks seem not to be working; maybe I need to turn on some debugging. This used to work at some point. Feb 2 23:51:28 infinity kernel: ACPI: \_SB_.PCI0.IDE1.PRI_.MAST: docking Feb 2 23:51:28 infinity kernel: ACPI: \_SB_.PCI0.IDE1.PRI_.MAST: Unable to dock! -- 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
Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -52,6 +52,7 @@ static const struct acpi_device_id acpi_ static LIST_HEAD(acpi_device_list); static LIST_HEAD(acpi_bus_id_list); +static DEFINE_MUTEX(acpi_scan_lock); DEFINE_MUTEX(acpi_device_lock); LIST_HEAD(acpi_wakeup_device_list); @@ -1612,19 +1613,22 @@ static acpi_status acpi_bus_device_attac int acpi_bus_scan(acpi_handle handle) { void *device = NULL; + int error = 0; + + mutex_lock(&acpi_scan_lock); if (ACPI_SUCCESS(acpi_bus_check_add(handle, 0, NULL, &device))) acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, acpi_bus_check_add, NULL, NULL, &device); if (!device) - return -ENODEV; - - if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) + error = -ENODEV; + else if (ACPI_SUCCESS(acpi_bus_device_attach(handle, 0, NULL, NULL))) acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, acpi_bus_device_attach, NULL, NULL, NULL); - return 0; + mutex_unlock(&acpi_scan_lock); + return error; } EXPORT_SYMBOL(acpi_bus_scan); @@ -1653,6 +1657,8 @@ static acpi_status acpi_bus_remove(acpi_ void acpi_bus_trim(struct acpi_device *start) { + mutex_lock(&acpi_scan_lock); + /* * Execute acpi_bus_device_detach() as a post-order callback to detach * all ACPI drivers from the device nodes being removed. @@ -1667,6 +1673,8 @@ void acpi_bus_trim(struct acpi_device *s acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL, acpi_bus_remove, NULL, NULL); acpi_bus_remove(start->handle, 0, NULL, NULL); + + mutex_unlock(&acpi_scan_lock); } EXPORT_SYMBOL_GPL(acpi_bus_trim);