Re: [BUG] WARN_ON(!context) in drivers/pci/hotplug/acpiphp_glue.c

[Bjorn Helgaas]

On Fri, Oct 11, 2013 at 7:01 AM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Fri, 11 Oct 2013 13:13:47 +0200
> "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx> wrote:
>>
>> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>> Subject: ACPI / hotplug / PCI: Accept coexistence with native PCIe hotplug
>>
>> Allow ACPIPHP (ACPI-based PCI hotplug) to handle event signaling for
>> devices that have already been claimed by the native PCIe hotplug
>> (pciehp).
>>
>> The ACPI hotplug events are essentially re-scan, remove and eject
>> requests.  Re-scan and remove should work regardless, because they
>> may be triggered by user space via sysfs and the ACPI eject (_EJ0)
>> should work if the BIOS wants us to use it.  There may be an issue
>> if the BIOS signals ACPI eject and wants us to use the native eject,
>> but that doesn't work without this change anyway.
>>
>> This change prevents the WARN_ON() in acpiphp_enumerate_slots() from
>> triggering unnecessarily for bridges whose parents are managed by
>> pciehp.
>>
>
> Reported-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Tested-by: Steven Rostedt <rostedt@xxxxxxxxxxx>

I opened https://bugzilla.kernel.org/show_bug.cgi?id=62831 for this
issue because I don't think this question of how to coordinate acpiphp
and pciehp is completely resolved, and I think it might be interesting
to have the complete dmesg, acpidump, and "lspci -vv" output attached
there for future reference.

Steve, I tried to attach the dmesg you mentioned yesterday in IRC, but
the link didn't work for me.  Would you mind attaching this info?

Rafael, I assume you'll probably merge this through your tree.  Would
you mind adding a reference to this bugzilla in the changelog?  I do
have a "convert to dynamic debug" acpiphp patch in my "next" branch
(bd950799), but I suspect you have several more interesting ones in
your tree.

Bjorn

>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>> ---
>>  drivers/pci/hotplug/acpiphp_glue.c |   32 ++++++++++++++++++++++++++------
>>  1 file changed, 26 insertions(+), 6 deletions(-)
>>
>> Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
>> ===================================================================
>> --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
>> +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
>> @@ -259,6 +259,31 @@ static void acpiphp_dock_release(void *d
>>       put_bridge(context->func.parent);
>>  }
>>
>> +/**
>> + * slot_should_be_exposed - Check whether or not to expose a slot to userland.
>> + * @bridge: ACPIPHP bridge the slot belongs to.
>> + * @handle: ACPI handle of a device in the slot.
>> + */
>> +static inline bool slot_should_be_exposed(struct acpiphp_bridge *bridge,
>> +                                       acpi_handle handle)
>> +{
>> +     struct pci_bus *pbus = bridge->pci_bus;
>> +     struct pci_dev *pdev = bridge->pci_dev;
>> +
>> +     /*
>> +      * Do not expose slots whose bridges are managed by pciehp, because they
>> +      * will be exposed to user space by the pciehp driver.
>> +      */
>> +     if (pdev && device_is_managed_by_native_pciehp(pdev))
>> +             return false;
>> +
>> +     /*
>> +      * Expose slots for devices with either _EJ0 or _RMV and for devices
>> +      * on docking stations.
>> +      */
>> +     return acpi_pci_check_ejectable(pbus, handle) || is_dock_device(handle);
>> +}
>> +
>>  /* callback routine to register each ACPI PCI slot object */
>>  static acpi_status register_slot(acpi_handle handle, u32 lvl, void *data,
>>                                void **rv)
>> @@ -271,12 +296,8 @@ static acpi_status register_slot(acpi_ha
>>       unsigned long long adr;
>>       int device, function;
>>       struct pci_bus *pbus = bridge->pci_bus;
>> -     struct pci_dev *pdev = bridge->pci_dev;
>>       u32 val;
>>
>> -     if (pdev && device_is_managed_by_native_pciehp(pdev))
>> -             return AE_OK;
>> -
>>       status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
>>       if (ACPI_FAILURE(status)) {
>>               acpi_handle_warn(handle, "can't evaluate _ADR (%#x)\n", status);
>> @@ -325,8 +346,7 @@ static acpi_status register_slot(acpi_ha
>>
>>       list_add_tail(&slot->node, &bridge->slots);
>>
>> -     /* Register slots for ejectable funtions only. */
>> -     if (acpi_pci_check_ejectable(pbus, handle)  || is_dock_device(handle)) {
>> +     if (slot_should_be_exposed(bridge, handle)) {
>>               unsigned long long sun;
>>               int retval;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/