On Thursday, September 05, 2013 09:44:26 PM Rafael J. Wysocki wrote:
> On Thursday, September 05, 2013 08:21:41 AM Alex Williamson wrote:
[...]
> > >
> > > [ 18.288122] pci 0000:00:00.0: no hotplug settings from platform
> > > [ 18.288127] pcieport 0000:00:01.0: no hotplug settings from platform
> > > [ 18.288142] pci 0000:01:00.0: no hotplug settings from platform
> > > [ 18.288157] pci 0000:01:00.1: no hotplug settings from platform
> > > [ 18.288162] pcieport 0000:00:03.0: no hotplug settings from platform
> > > [ 18.288176] pci 0000:02:00.0: no hotplug settings from platform
> > > [ 18.288190] pci 0000:02:00.1: no hotplug settings from platform
> > > [ 18.288195] pcieport 0000:00:07.0: no hotplug settings from platform
> > > [ 18.288209] pci 0000:03:00.0: no hotplug settings from platform
> > > [ 18.288224] pci 0000:03:00.1: no hotplug settings from platform
> > > [ 18.288228] pci 0000:00:14.0: no hotplug settings from platform
> > > [ 18.288233] pci 0000:00:14.1: no hotplug settings from platform
> > > [ 18.288237] pci 0000:00:14.2: no hotplug settings from platform
> > > [ 18.288242] pci 0000:00:16.0: no hotplug settings from platform
> > > [ 18.288247] pci 0000:00:16.1: no hotplug settings from platform
> > > [ 18.288251] pci 0000:00:16.2: no hotplug settings from platform
> > > [ 18.288256] pci 0000:00:16.3: no hotplug settings from platform
> > > [ 18.288260] pci 0000:00:16.4: no hotplug settings from platform
> > > [ 18.288265] pci 0000:00:16.5: no hotplug settings from platform
> > > [ 18.288269] pci 0000:00:16.6: no hotplug settings from platform
> > > [ 18.288274] pci 0000:00:16.7: no hotplug settings from platform
> > > [ 18.288278] pci 0000:00:1a.0: no hotplug settings from platform
> > > [ 18.288279] pci 0000:00:1a.0: using default PCI settings
> > > [ 18.288292] pci 0000:00:1a.1: no hotplug settings from platform
> > > [ 18.288293] pci 0000:00:1a.1: using default PCI settings
> > > [ 18.288307] ehci-pci 0000:00:1a.7: no hotplug settings from platform
> > > [ 18.288308] ehci-pci 0000:00:1a.7: using default PCI settings
> > > [ 18.288322] pci 0000:00:1b.0: no hotplug settings from platform
> > > [ 18.288327] pcieport 0000:00:1c.0: no hotplug settings from platform
> > > [ 18.288332] pcieport 0000:00:1c.4: no hotplug settings from platform
> > > [ 18.288344] pci 0000:05:00.0: no hotplug settings from platform
> > > [ 18.288349] pci 0000:00:1d.0: no hotplug settings from platform
> > > [ 18.288350] pci 0000:00:1d.0: using default PCI settings
> > > [ 18.288360] pci 0000:00:1d.1: no hotplug settings from platform
> > > [ 18.288361] pci 0000:00:1d.1: using default PCI settings
> > > [ 18.288374] pci 0000:00:1d.2: no hotplug settings from platform
> > > [ 18.288374] pci 0000:00:1d.2: using default PCI settings
> > > [ 18.288387] pci 0000:00:1d.3: no hotplug settings from platform
> > > [ 18.288387] pci 0000:00:1d.3: using default PCI settings
> > >
> > > The boot is noticeably slower. What's going to happen on systems that
> > > actually have a significant I/O topology vs my little workstation?
>
> That depends on how many bus check/device check events they generate on boot.
>
> My test machines don't generate them during boot at all (even the one with
> a Thunderbolt connector), so I don't see the messages in question during boot
> on any of them. Mika doesn't see them either I suppose, or he would have told
> me about that before.
>
> And let's just make it clear that it is not usual or even OK to generate bus
> checks or device checks during boot like this. And since the changes in
> question have been in linux-next since right after the 3.11 merge window, I
> think that someone would have complained already had that been a common issue.
>
> Of course, we need to deal with that somehow nevertheless. :-)
>
> > Just to give you an idea:
> >
> > CONFIG_HOTPLUG_PCI_ACPI=y
> >
> > $ dmesg | wc
> > 5697 49935 384368
> >
> > $ dmesg | tail --lines=1
> > [ 53.137123] Ebtables v2.0 registered
> >
> > -- vs --
> >
> > # CONFIG_HOTPLUG_PCI_ACPI is not set
> >
> > $ dmesg | wc
> > 1053 9176 71652
> >
> > $dmesg | tail --lines=1
> > [ 28.917220] Ebtables v2.0 registered
> >
> > So it spews out 5x more output with acpiphp enabled and takes and extra
> > 24s to boot (nearly 2x). Not good.
>
> The "no hotplug settings from platform" message is from pci_configure_slot().
> I think the messages you're seeing are from the call to it in
> acpiphp_set_hpp_values() which is called by enable_slot().
>
> There, I think, we can simply check the return value of pci_scan_slot() and
> if that is 0 (no new devices), we can just skip everything under the call to
> __pci_bus_assign_resources().
>
> However, we can't skip the scanning of bridges, if any, because there may be
> new devices below them and I guess that's what takes so much time on your
> machine.
OK, one piece is missing. We may need to evaluate _OSC after handling each
event to let the platform know the status.
Can you please check if the appended patch makes any difference (with the
previous fix applied, of course)?
If fact, it is two patches combined. One of them optimizes enable_slot()
slightly and the other adds the missing _OSC evaluation.
Thanks,
Rafael
---
drivers/pci/hotplug/acpiphp_glue.c | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 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
@@ -542,12 +542,12 @@ static void __ref enable_slot(struct acp
struct acpiphp_func *func;
int max, pass;
LIST_HEAD(add_list);
+ int nr_found;
list_for_each_entry(func, &slot->funcs, sibling)
acpiphp_bus_add(func_to_handle(func));
- pci_scan_slot(bus, PCI_DEVFN(slot->device, 0));
-
+ nr_found = pci_scan_slot(bus, PCI_DEVFN(slot->device, 0));
max = acpiphp_max_busnr(bus);
for (pass = 0; pass < 2; pass++) {
list_for_each_entry(dev, &bus->devices, bus_list) {
@@ -566,8 +566,11 @@ static void __ref enable_slot(struct acp
}
}
}
-
__pci_bus_assign_resources(bus, &add_list, NULL);
+ /* Nothing more to do here if there are no new devices on this bus. */
+ if (!nr_found && (slot->flags & SLOT_ENABLED))
+ return;
+
acpiphp_sanitize_bus(bus);
acpiphp_set_hpp_values(bus);
acpiphp_set_acpi_region(slot);
@@ -867,6 +870,8 @@ static void hotplug_event_work(struct wo
hotplug_event(hp_work->handle, hp_work->type, context);
acpi_scan_lock_release();
+ acpi_evaluate_hotplug_ost(hp_work->handle, hp_work->type,
+ ACPI_OST_SC_SUCCESS, NULL);
kfree(hp_work); /* allocated in handle_hotplug_event() */
put_bridge(context->func.parent);
}
@@ -882,12 +887,16 @@ static void hotplug_event_work(struct wo
static void handle_hotplug_event(acpi_handle handle, u32 type, void *data)
{
struct acpiphp_context *context;
+ u32 ost_code;
switch (type) {
case ACPI_NOTIFY_BUS_CHECK:
case ACPI_NOTIFY_DEVICE_CHECK:
+ ost_code = ACPI_OST_SC_INSERT_IN_PROGRESS;
+ goto work;
case ACPI_NOTIFY_EJECT_REQUEST:
- break;
+ ost_code = ACPI_OST_SC_EJECT_IN_PROGRESS;
+ goto work;
case ACPI_NOTIFY_DEVICE_WAKE:
return;
@@ -895,30 +904,40 @@ static void handle_hotplug_event(acpi_ha
case ACPI_NOTIFY_FREQUENCY_MISMATCH:
acpi_handle_err(handle, "Device cannot be configured due "
"to a frequency mismatch\n");
- return;
+ break;
case ACPI_NOTIFY_BUS_MODE_MISMATCH:
acpi_handle_err(handle, "Device cannot be configured due "
"to a bus mode mismatch\n");
- return;
+ break;
case ACPI_NOTIFY_POWER_FAULT:
acpi_handle_err(handle, "Device has suffered a power fault\n");
- return;
+ break;
default:
acpi_handle_warn(handle, "Unsupported event type 0x%x\n", type);
- return;
+ break;
}
+ err:
+ ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
+ acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
+ return;
+
+ work:
mutex_lock(&acpiphp_context_lock);
context = acpiphp_get_context(handle);
if (context) {
get_bridge(context->func.parent);
acpiphp_put_context(context);
+ acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
alloc_acpi_hp_work(handle, type, context, hotplug_event_work);
+ mutex_unlock(&acpiphp_context_lock);
+ return;
}
mutex_unlock(&acpiphp_context_lock);
+ goto err;
}
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
