Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1410601 > unrolled thread

Regression in IO resource allocation

Back to article view | Back to linux.kernel

Contents

  Regression in IO resource allocation Roland Dreier <roland@purestorage.com> - 2016-05-31 22:20 +0200
    Re: Regression in IO resource allocation "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-05-31 23:10 +0200
      Re: Regression in IO resource allocation Roland Dreier <roland@purestorage.com> - 2016-05-31 23:50 +0200
        Re: Regression in IO resource allocation "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-01 00:40 +0200
          Re: Regression in IO resource allocation "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-01 00:40 +0200
          Re: Regression in IO resource allocation Roland Dreier <roland@purestorage.com> - 2016-06-01 19:10 +0200
            Re: Regression in IO resource allocation "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-06-01 22:20 +0200

#1410601 — Regression in IO resource allocation

Hi,

I recently updated one of my systems from 3.10.y to 4.4.11, and
discovered a regression that stops it from booting.  It's actually
very similar to https://bugzilla.kernel.org/show_bug.cgi?id=99831
(which I reported about the same system last year).

The problem is that commit ac212b6980d8 ("ACPI / processor: Use common
hotplug infrastructure") changes the order that the ACPI processor and
PnP initialization run.  pnp_system_init() is run at fs_initcall time,
while acpi_processor_init() is run from acpi_scan_init(), earlier at
subsys_initcall time.  Pre-ac212b6980d8, the ACPI processor
initialization all ran from acpi_processor_init() at module_init time.
So the processor driver initialization has flipped from after to
before pnp_system_init().

Just as before, the failure is that the resource allocation code puts
some AHCI IO BARs around 0x400, and reservation fails because some
other ACPI stuff is also there.  The problem is that when acpi_processor_init()
runs, it reserves a range 0x410 - 0x415 for "ACPI CPU throttle", and
if that happens before pnp_system_init(), then I get

    system 00:01: [io  0x0400-0x047f] could not be reserved

because that overlaps the already-reserved range.  Then the PCI
resource allocation code is free to put PCI resources into that range
and tons of things go south after that.

For now I've worked around it by commenting out the request_region()
in acpi_processor.c but that doesn't seem like a very good long-term
solution.  Does it make sense to resurrect the patches you had to let
ACPI and PnP coexist in resource reservation?  Or could we move the
request_region() for CPU throttle into the still-modular
initialization done from acpi_processor_driver_init()?

Thanks!
  Roland

[toc] | [next] | [standalone]

#1410626

On Tuesday, May 31, 2016 01:12:52 PM Roland Dreier wrote:
> Hi,
> 
> I recently updated one of my systems from 3.10.y to 4.4.11, and
> discovered a regression that stops it from booting.  It's actually
> very similar to https://bugzilla.kernel.org/show_bug.cgi?id=99831
> (which I reported about the same system last year).
> 
> The problem is that commit ac212b6980d8 ("ACPI / processor: Use common
> hotplug infrastructure") changes the order that the ACPI processor and
> PnP initialization run.  pnp_system_init() is run at fs_initcall time,
> while acpi_processor_init() is run from acpi_scan_init(), earlier at
> subsys_initcall time.  Pre-ac212b6980d8, the ACPI processor
> initialization all ran from acpi_processor_init() at module_init time.
> So the processor driver initialization has flipped from after to
> before pnp_system_init().
> 
> Just as before, the failure is that the resource allocation code puts
> some AHCI IO BARs around 0x400, and reservation fails because some
> other ACPI stuff is also there.  The problem is that when acpi_processor_init()
> runs, it reserves a range 0x410 - 0x415 for "ACPI CPU throttle", and
> if that happens before pnp_system_init(), then I get
> 
>     system 00:01: [io  0x0400-0x047f] could not be reserved
> 
> because that overlaps the already-reserved range.  Then the PCI
> resource allocation code is free to put PCI resources into that range
> and tons of things go south after that.

Definitely the request_region() in acpi_processor.c is a bug as that file
should be about enumeration only (and we don't even know whether or not
the region will be actually used at that point).

> For now I've worked around it by commenting out the request_region()
> in acpi_processor.c but that doesn't seem like a very good long-term
> solution.  Does it make sense to resurrect the patches you had to let
> ACPI and PnP coexist in resource reservation?  Or could we move the
> request_region() for CPU throttle into the still-modular
> initialization done from acpi_processor_driver_init()?

In ptinciple, that can be done.

Can you please try the appended patch (untested)?

Thanks,
Rafael


---
 drivers/acpi/acpi_processor.c       |    9 ---------
 drivers/acpi/processor_throttling.c |    9 +++++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

Index: linux-pm/drivers/acpi/acpi_processor.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_processor.c
+++ linux-pm/drivers/acpi/acpi_processor.c
@@ -331,15 +331,6 @@ static int acpi_processor_get_info(struc
 		pr->throttling.duty_width = acpi_gbl_FADT.duty_width;
 
 		pr->pblk = object.processor.pblk_address;
-
-		/*
-		 * We don't care about error returns - we just try to mark
-		 * these reserved so that nobody else is confused into thinking
-		 * that this region might be unused..
-		 *
-		 * (In particular, allocating the IO range for Cardbus)
-		 */
-		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
 	}
 
 	/*
Index: linux-pm/drivers/acpi/processor_throttling.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_throttling.c
+++ linux-pm/drivers/acpi/processor_throttling.c
@@ -676,6 +676,15 @@ static int acpi_processor_get_throttling
 	if (!pr->flags.throttling)
 		return -ENODEV;
 
+	/*
+	 * We don't care about error returns - we just try to mark
+	 * these reserved so that nobody else is confused into thinking
+	 * that this region might be unused..
+	 *
+	 * (In particular, allocating the IO range for Cardbus)
+	 */
+	request_region(pr->throttling.address, 6, "ACPI CPU throttle");
+
 	pr->throttling.state = 0;
 
 	duty_mask = pr->throttling.state_count - 1;

[toc] | [prev] | [next] | [standalone]

#1410654

On Tue, May 31, 2016 at 2:11 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> Can you please try the appended patch (untested)?

Thanks for the quick reply.  Patch looks OK on my system... it boots
(which is very good :) and I see

    system 00:01: [io  0x0400-0x047f] has been reserved

however I don't see the "ACPI CPU throttle" region reserved in
/proc/ioports... haven't debugged why acpi_processor_get_throttling()
isn't getting called or what is happening yet.

Will dig a bit deeper and let you know.

 - R.

[toc] | [prev] | [next] | [standalone]

#1410666

On Tue, May 31, 2016 at 11:42 PM, Roland Dreier <roland@purestorage.com> wrote:
> On Tue, May 31, 2016 at 2:11 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> Can you please try the appended patch (untested)?
>
> Thanks for the quick reply.  Patch looks OK on my system... it boots
> (which is very good :) and I see
>
>     system 00:01: [io  0x0400-0x047f] has been reserved
>
> however I don't see the "ACPI CPU throttle" region reserved in
> /proc/ioports... haven't debugged why acpi_processor_get_throttling()
> isn't getting called or what is happening yet.
>
> Will dig a bit deeper and let you know.

It may not be called at all if _PTC is used on that system, for example.

[toc] | [prev] | [next] | [standalone]

#1410667

On Wed, Jun 1, 2016 at 12:31 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, May 31, 2016 at 11:42 PM, Roland Dreier <roland@purestorage.com> wrote:
>> On Tue, May 31, 2016 at 2:11 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>> Can you please try the appended patch (untested)?
>>
>> Thanks for the quick reply.  Patch looks OK on my system... it boots
>> (which is very good :) and I see
>>
>>     system 00:01: [io  0x0400-0x047f] has been reserved
>>
>> however I don't see the "ACPI CPU throttle" region reserved in
>> /proc/ioports... haven't debugged why acpi_processor_get_throttling()
>> isn't getting called or what is happening yet.
>>
>> Will dig a bit deeper and let you know.
>
> It may not be called at all if _PTC is used on that system, for example.

I mean acpi_processor_get_throttling_fadt(), of course. :-)

[toc] | [prev] | [next] | [standalone]

#1411412

On Tue, May 31, 2016 at 3:31 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> It may not be called at all if _PTC is used on that system, for example.

Yes, that's exactly the case on my system.

So from my POV:

Tested-by: Roland Dreier <roland@purestorage.com>

Thanks!

[toc] | [prev] | [next] | [standalone]

#1411527

On Wednesday, June 01, 2016 10:08:59 AM Roland Dreier wrote:
> On Tue, May 31, 2016 at 3:31 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > It may not be called at all if _PTC is used on that system, for example.
> 
> Yes, that's exactly the case on my system.
> 
> So from my POV:
> 
> Tested-by: Roland Dreier <roland@purestorage.com>
> 
> Thanks!

OK, I'll queue it up, then.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web