* [PATCH 0/2] ACPI / scan: Additional changes
@ 2013-01-26 22:39 Rafael J. Wysocki
2013-01-26 22:41 ` [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive Rafael J. Wysocki
2013-01-26 22:43 ` [PATCH 2/2] ACPI / scan: Make scanning of fixed devices follow the general scheme Rafael J. Wysocki
0 siblings, 2 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-26 22:39 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: LKML, Bjorn Helgaas, Yinghai Lu, Jiang Liu, Toshi Kani
Hi All,
The following patches make two more changes to the ACPI namespace scanning
code that I think are more-or-less useful:
[1/2] Introduce lock to prevent acpi_bus_scan() from running in parallel with
acpi_bus_trim() to avoid removing device nodes while we're setting
them up.
[2/2] Change the scanning of fixed device nodes to work along the lines of
acpi_bus_scan().
Patch [1/2] should apply on top of the acpi-scan branch, but [2/2] needs the
entire linux-pm.git/linux-next branch.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-01-26 22:39 [PATCH 0/2] ACPI / scan: Additional changes Rafael J. Wysocki @ 2013-01-26 22:41 ` Rafael J. Wysocki 2013-01-26 23:19 ` Yinghai Lu 2013-01-26 22:43 ` [PATCH 2/2] ACPI / scan: Make scanning of fixed devices follow the general scheme Rafael J. Wysocki 1 sibling, 1 reply; 11+ messages in thread From: Rafael J. Wysocki @ 2013-01-26 22:41 UTC (permalink / raw) To: ACPI Devel Maling List Cc: LKML, Bjorn Helgaas, Yinghai Lu, Jiang Liu, Toshi Kani 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> --- 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); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-01-26 22:41 ` [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive Rafael J. Wysocki @ 2013-01-26 23:19 ` Yinghai Lu 2013-01-29 13:59 ` Steven Newbury 2013-02-02 11:58 ` Steven Newbury 0 siblings, 2 replies; 11+ messages in thread From: Yinghai Lu @ 2013-01-26 23:19 UTC (permalink / raw) To: Rafael J. Wysocki, Steven Newbury Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani 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); > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-01-26 23:19 ` Yinghai Lu @ 2013-01-29 13:59 ` Steven Newbury 2013-02-02 11:58 ` Steven Newbury 1 sibling, 0 replies; 11+ messages in thread From: Steven Newbury @ 2013-01-29 13:59 UTC (permalink / raw) To: Yinghai Lu Cc: Rafael J. Wysocki, ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani 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); > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-01-26 23:19 ` Yinghai Lu 2013-01-29 13:59 ` Steven Newbury @ 2013-02-02 11:58 ` Steven Newbury 2013-02-02 20:18 ` Rafael J. Wysocki 1 sibling, 1 reply; 11+ messages in thread From: Steven Newbury @ 2013-02-02 11:58 UTC (permalink / raw) To: Yinghai Lu Cc: Rafael J. Wysocki, ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani 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); > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-02-02 11:58 ` Steven Newbury @ 2013-02-02 20:18 ` Rafael J. Wysocki 2013-02-02 20:28 ` Steven Newbury 0 siblings, 1 reply; 11+ messages in thread From: Rafael J. Wysocki @ 2013-02-02 20:18 UTC (permalink / raw) To: Steven Newbury Cc: Yinghai Lu, ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani 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 -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-02-02 20:18 ` Rafael J. Wysocki @ 2013-02-02 20:28 ` Steven Newbury 2013-02-02 22:27 ` Rafael J. Wysocki 0 siblings, 1 reply; 11+ messages in thread From: Steven Newbury @ 2013-02-02 20:28 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Yinghai Lu, ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-02-02 20:28 ` Steven Newbury @ 2013-02-02 22:27 ` Rafael J. Wysocki 2013-02-03 8:45 ` Steven Newbury 0 siblings, 1 reply; 11+ messages in thread From: Rafael J. Wysocki @ 2013-02-02 22:27 UTC (permalink / raw) To: Steven Newbury Cc: Yinghai Lu, ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani 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 -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive 2013-02-02 22:27 ` Rafael J. Wysocki @ 2013-02-03 8:45 ` Steven Newbury 0 siblings, 0 replies; 11+ messages in thread From: Steven Newbury @ 2013-02-03 8:45 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Yinghai Lu, ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani 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! ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ACPI / scan: Make scanning of fixed devices follow the general scheme 2013-01-26 22:39 [PATCH 0/2] ACPI / scan: Additional changes Rafael J. Wysocki 2013-01-26 22:41 ` [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive Rafael J. Wysocki @ 2013-01-26 22:43 ` Rafael J. Wysocki 2013-01-26 23:26 ` Yinghai Lu 1 sibling, 1 reply; 11+ messages in thread From: Rafael J. Wysocki @ 2013-01-26 22:43 UTC (permalink / raw) To: ACPI Devel Maling List Cc: LKML, Bjorn Helgaas, Yinghai Lu, Jiang Liu, Toshi Kani From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Make acpi_bus_scan_fixed() use device_attach() directly to attach drivers, if any, to the fixed devices in analogy with how acpi_bus_scan() works, which allows the last argument of acpi_add_single_object() to be dropped and the manipulation of the flags.match_driver bit to be moved to acpi_init_device_object() and acpi_device_add_finalize(). After these changes all of the functions for the initialization and registration of struct acpi_device objects work in the same way for all of them. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/scan.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -1437,19 +1437,21 @@ void acpi_init_device_object(struct acpi acpi_device_get_busid(device); acpi_device_set_id(device); acpi_bus_get_flags(device); + device->flags.match_driver = false; device_initialize(&device->dev); dev_set_uevent_suppress(&device->dev, true); } void acpi_device_add_finalize(struct acpi_device *device) { + device->flags.match_driver = true; dev_set_uevent_suppress(&device->dev, false); kobject_uevent(&device->dev.kobj, KOBJ_ADD); } static int acpi_add_single_object(struct acpi_device **child, acpi_handle handle, int type, - unsigned long long sta, bool match_driver) + unsigned long long sta) { int result; struct acpi_device *device; @@ -1465,7 +1467,6 @@ static int acpi_add_single_object(struct acpi_bus_get_power_flags(device); acpi_bus_get_wakeup_device_flags(device); - device->flags.match_driver = match_driver; result = acpi_device_add(device, acpi_device_release); if (result) { acpi_device_release(&device->dev); @@ -1558,12 +1559,10 @@ static acpi_status acpi_bus_check_add(ac return AE_CTRL_DEPTH; } - acpi_add_single_object(&device, handle, type, sta, false); + acpi_add_single_object(&device, handle, type, sta); if (!device) return AE_CTRL_DEPTH; - device->flags.match_driver = true; - out: if (!*return_value) *return_value = device; @@ -1681,25 +1680,39 @@ EXPORT_SYMBOL_GPL(acpi_bus_trim); static int acpi_bus_scan_fixed(void) { int result = 0; - struct acpi_device *device = NULL; /* * Enumerate all fixed-feature devices. */ - if ((acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON) == 0) { + if (!(acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON)) { + struct acpi_device *device = NULL; + result = acpi_add_single_object(&device, NULL, ACPI_BUS_TYPE_POWER_BUTTON, - ACPI_STA_DEFAULT, true); + ACPI_STA_DEFAULT); + if (result) + return result; + + result = device_attach(&device->dev); + if (result < 0) + return result; + device_init_wakeup(&device->dev, true); } - if ((acpi_gbl_FADT.flags & ACPI_FADT_SLEEP_BUTTON) == 0) { + if (!(acpi_gbl_FADT.flags & ACPI_FADT_SLEEP_BUTTON)) { + struct acpi_device *device = NULL; + result = acpi_add_single_object(&device, NULL, ACPI_BUS_TYPE_SLEEP_BUTTON, - ACPI_STA_DEFAULT, true); + ACPI_STA_DEFAULT); + if (result) + return result; + + result = device_attach(&device->dev); } - return result; + return result < 0 ? result : 0; } int __init acpi_scan_init(void) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ACPI / scan: Make scanning of fixed devices follow the general scheme 2013-01-26 22:43 ` [PATCH 2/2] ACPI / scan: Make scanning of fixed devices follow the general scheme Rafael J. Wysocki @ 2013-01-26 23:26 ` Yinghai Lu 0 siblings, 0 replies; 11+ messages in thread From: Yinghai Lu @ 2013-01-26 23:26 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Jiang Liu, Toshi Kani On Sat, Jan 26, 2013 at 2:43 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make acpi_bus_scan_fixed() use device_attach() directly to attach > drivers, if any, to the fixed devices in analogy with how > acpi_bus_scan() works, which allows the last argument of > acpi_add_single_object() to be dropped and the manipulation of the > flags.match_driver bit to be moved to acpi_init_device_object() > and acpi_device_add_finalize(). > > After these changes all of the functions for the initialization > and registration of struct acpi_device objects work in the same > way for all of them. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/scan.c | 35 ++++++++++++++++++++++++----------- > 1 file changed, 24 insertions(+), 11 deletions(-) > > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -1437,19 +1437,21 @@ void acpi_init_device_object(struct acpi > acpi_device_get_busid(device); > acpi_device_set_id(device); > acpi_bus_get_flags(device); > + device->flags.match_driver = false; > device_initialize(&device->dev); > dev_set_uevent_suppress(&device->dev, true); > } > > void acpi_device_add_finalize(struct acpi_device *device) > { > + device->flags.match_driver = true; > dev_set_uevent_suppress(&device->dev, false); > kobject_uevent(&device->dev.kobj, KOBJ_ADD); > } > > static int acpi_add_single_object(struct acpi_device **child, > acpi_handle handle, int type, > - unsigned long long sta, bool match_driver) > + unsigned long long sta) > { > int result; > struct acpi_device *device; > @@ -1465,7 +1467,6 @@ static int acpi_add_single_object(struct > acpi_bus_get_power_flags(device); > acpi_bus_get_wakeup_device_flags(device); > > - device->flags.match_driver = match_driver; > result = acpi_device_add(device, acpi_device_release); > if (result) { > acpi_device_release(&device->dev); > @@ -1558,12 +1559,10 @@ static acpi_status acpi_bus_check_add(ac > return AE_CTRL_DEPTH; > } > > - acpi_add_single_object(&device, handle, type, sta, false); > + acpi_add_single_object(&device, handle, type, sta); > if (!device) > return AE_CTRL_DEPTH; > > - device->flags.match_driver = true; > - > out: > if (!*return_value) > *return_value = device; nice cleanup. Acked-by: Yinghai Lu <yinghai@kernel.org> > @@ -1681,25 +1680,39 @@ EXPORT_SYMBOL_GPL(acpi_bus_trim); > static int acpi_bus_scan_fixed(void) > { > int result = 0; > - struct acpi_device *device = NULL; > > /* > * Enumerate all fixed-feature devices. > */ > - if ((acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON) == 0) { > + if (!(acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON)) { > + struct acpi_device *device = NULL; > + > result = acpi_add_single_object(&device, NULL, > ACPI_BUS_TYPE_POWER_BUTTON, > - ACPI_STA_DEFAULT, true); > + ACPI_STA_DEFAULT); > + if (result) > + return result; > + > + result = device_attach(&device->dev); > + if (result < 0) > + return result; > + > device_init_wakeup(&device->dev, true); > } > > - if ((acpi_gbl_FADT.flags & ACPI_FADT_SLEEP_BUTTON) == 0) { > + if (!(acpi_gbl_FADT.flags & ACPI_FADT_SLEEP_BUTTON)) { > + struct acpi_device *device = NULL; > + > result = acpi_add_single_object(&device, NULL, > ACPI_BUS_TYPE_SLEEP_BUTTON, > - ACPI_STA_DEFAULT, true); > + ACPI_STA_DEFAULT); > + if (result) > + return result; > + > + result = device_attach(&device->dev); > } > > - return result; > + return result < 0 ? result : 0; > } > > int __init acpi_scan_init(void) > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-02-03 8:45 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-26 22:39 [PATCH 0/2] ACPI / scan: Additional changes Rafael J. Wysocki 2013-01-26 22:41 ` [PATCH 1/2] ACPI / scan: Make namespace scanning and trimming mutually exclusive Rafael J. Wysocki 2013-01-26 23:19 ` Yinghai Lu 2013-01-29 13:59 ` Steven Newbury 2013-02-02 11:58 ` Steven Newbury 2013-02-02 20:18 ` Rafael J. Wysocki 2013-02-02 20:28 ` Steven Newbury 2013-02-02 22:27 ` Rafael J. Wysocki 2013-02-03 8:45 ` Steven Newbury 2013-01-26 22:43 ` [PATCH 2/2] ACPI / scan: Make scanning of fixed devices follow the general scheme Rafael J. Wysocki 2013-01-26 23:26 ` Yinghai Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).