Re: Linux 4.15-rc2

[Eugene Syromiatnikov]

On Sun, Dec 03, 2017 at 11:22:56AM -0500, Linus Torvalds wrote:
> 
> Linus Torvalds (6):
>   Rename superblock flags (MS_xyz -> SB_xyz)

This commit, while claims that it changes internal flags, also touches
an UAPI header (include/uapi/linux/bfs_fs.h), specifically, the macro
BFS_UNCLEAN.  I expect that either this macro should be in a private
header, or (if this check is expected to be available to the userspace)
at least SB_RDONLY should be also defined there.

Re: Linux 4.15-rc2

[Eugene Syromiatnikov]

On Sun, Dec 03, 2017 at 11:22:56AM -0500, Linus Torvalds wrote:
> 
> Linus Torvalds (6):
>   Rename superblock flags (MS_xyz -> SB_xyz)

This commit, while claims that it changes internal flags, also touches
an UAPI header (include/uapi/linux/bfs_fs.h), specifically, the macro
BFS_UNCLEAN.  I expect that either this macro should be in a private
header, or (if this check is expected to be available to the userspace)
at least SB_RDONLY should be also defined there.

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Linus Torvalds wrote:

> On Thu, Dec 14, 2017 at 2:36 PM, Thomas Gleixner  wrote:
> >
> > But, because the silly firmware comes out of suspend with all PIC lines
> > unmasked for whatever reason, the PIC can observe that IRQ being raised and
> > the CPU not handling it. So yes, I forgot about 7 being magic, but I still
> > think it's the firmware which causes it by unmasking the PIC irqs.
> 
> Yes, that sounds quite likely.

And just for the record I was able to figure out which interrupt comes in
and goes away again. It's the only level triggered interrupt, which is the
ACPI interrupt.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Linus Torvalds wrote:

> On Thu, Dec 14, 2017 at 2:36 PM, Thomas Gleixner  wrote:
> >
> > But, because the silly firmware comes out of suspend with all PIC lines
> > unmasked for whatever reason, the PIC can observe that IRQ being raised and
> > the CPU not handling it. So yes, I forgot about 7 being magic, but I still
> > think it's the firmware which causes it by unmasking the PIC irqs.
> 
> Yes, that sounds quite likely.

And just for the record I was able to figure out which interrupt comes in
and goes away again. It's the only level triggered interrupt, which is the
ACPI interrupt.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thu, Dec 14, 2017 at 11:36 PM, Thomas Gleixner  wrote:
> On Thu, 14 Dec 2017, Linus Torvalds wrote:
>> On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner  wrote:
>> I just wanted to pipe up about that "irq7", because judging from your
>> email it seems like you think it's a real irq:
>>
>> >Now there is a race
>> > whether the kernel resume path manages to mask the PIC again early enough
>> > before something triggers IRQ7 or not.
>>
>> ..and that's not how the PIC works.
>>
>> In fact, "legacy irq 7" is the _normal_ and very traditional spurious
>> interrupt, and it's documented. If the PIC gets an interrupt from
>> _any_ source, but the interrupt goes away before the PIC gets an
>> acknowledge from the CPU (and by "acknowledge", I'm not talking about
>> the explicit software IRQ ACK, I'm talking about the hardware
>> protocol, between the PIC and the CPU), the PIC will then report irq 7
>> as the interrupt - regardless of what the original was.
>>
>> The reason is almost always something like
>>
>>  - CPU interrupts are disabled or masked
>>
>>  - driver does a write to the external hardware that causes an
>> interrupt to be raised
>
> Which should be a non issue because _ALL_ PIC irq lines are masked at the
> PIC itself. All interrupts are routed through IOAPIC. So unless the IOAPIC
> sports similar behaviour the PIC should not ever observe that scenario.
>
> But, because the silly firmware comes out of suspend with all PIC lines
> unmasked for whatever reason, the PIC can observe that IRQ being raised and
> the CPU not handling it. So yes, I forgot about 7 being magic, but I still
> think it's the firmware which causes it by unmasking the PIC irqs.

That's my understanding too.

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thu, Dec 14, 2017 at 11:36 PM, Thomas Gleixner  wrote:
> On Thu, 14 Dec 2017, Linus Torvalds wrote:
>> On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner  wrote:
>> I just wanted to pipe up about that "irq7", because judging from your
>> email it seems like you think it's a real irq:
>>
>> >Now there is a race
>> > whether the kernel resume path manages to mask the PIC again early enough
>> > before something triggers IRQ7 or not.
>>
>> ..and that's not how the PIC works.
>>
>> In fact, "legacy irq 7" is the _normal_ and very traditional spurious
>> interrupt, and it's documented. If the PIC gets an interrupt from
>> _any_ source, but the interrupt goes away before the PIC gets an
>> acknowledge from the CPU (and by "acknowledge", I'm not talking about
>> the explicit software IRQ ACK, I'm talking about the hardware
>> protocol, between the PIC and the CPU), the PIC will then report irq 7
>> as the interrupt - regardless of what the original was.
>>
>> The reason is almost always something like
>>
>>  - CPU interrupts are disabled or masked
>>
>>  - driver does a write to the external hardware that causes an
>> interrupt to be raised
>
> Which should be a non issue because _ALL_ PIC irq lines are masked at the
> PIC itself. All interrupts are routed through IOAPIC. So unless the IOAPIC
> sports similar behaviour the PIC should not ever observe that scenario.
>
> But, because the silly firmware comes out of suspend with all PIC lines
> unmasked for whatever reason, the PIC can observe that IRQ being raised and
> the CPU not handling it. So yes, I forgot about 7 being magic, but I still
> think it's the firmware which causes it by unmasking the PIC irqs.

That's my understanding too.

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Thu, Dec 14, 2017 at 2:36 PM, Thomas Gleixner  wrote:
>
> But, because the silly firmware comes out of suspend with all PIC lines
> unmasked for whatever reason, the PIC can observe that IRQ being raised and
> the CPU not handling it. So yes, I forgot about 7 being magic, but I still
> think it's the firmware which causes it by unmasking the PIC irqs.

Yes, that sounds quite likely.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Thu, Dec 14, 2017 at 2:36 PM, Thomas Gleixner  wrote:
>
> But, because the silly firmware comes out of suspend with all PIC lines
> unmasked for whatever reason, the PIC can observe that IRQ being raised and
> the CPU not handling it. So yes, I forgot about 7 being magic, but I still
> think it's the firmware which causes it by unmasking the PIC irqs.

Yes, that sounds quite likely.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Linus Torvalds wrote:
> On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner  wrote:
> I just wanted to pipe up about that "irq7", because judging from your
> email it seems like you think it's a real irq:
> 
> >Now there is a race
> > whether the kernel resume path manages to mask the PIC again early enough
> > before something triggers IRQ7 or not.
> 
> ..and that's not how the PIC works.
> 
> In fact, "legacy irq 7" is the _normal_ and very traditional spurious
> interrupt, and it's documented. If the PIC gets an interrupt from
> _any_ source, but the interrupt goes away before the PIC gets an
> acknowledge from the CPU (and by "acknowledge", I'm not talking about
> the explicit software IRQ ACK, I'm talking about the hardware
> protocol, between the PIC and the CPU), the PIC will then report irq 7
> as the interrupt - regardless of what the original was.
> 
> The reason is almost always something like
> 
>  - CPU interrupts are disabled or masked
> 
>  - driver does a write to the external hardware that causes an
> interrupt to be raised

Which should be a non issue because _ALL_ PIC irq lines are masked at the
PIC itself. All interrupts are routed through IOAPIC. So unless the IOAPIC
sports similar behaviour the PIC should not ever observe that scenario.

But, because the silly firmware comes out of suspend with all PIC lines
unmasked for whatever reason, the PIC can observe that IRQ being raised and
the CPU not handling it. So yes, I forgot about 7 being magic, but I still
think it's the firmware which causes it by unmasking the PIC irqs.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Linus Torvalds wrote:
> On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner  wrote:
> I just wanted to pipe up about that "irq7", because judging from your
> email it seems like you think it's a real irq:
> 
> >Now there is a race
> > whether the kernel resume path manages to mask the PIC again early enough
> > before something triggers IRQ7 or not.
> 
> ..and that's not how the PIC works.
> 
> In fact, "legacy irq 7" is the _normal_ and very traditional spurious
> interrupt, and it's documented. If the PIC gets an interrupt from
> _any_ source, but the interrupt goes away before the PIC gets an
> acknowledge from the CPU (and by "acknowledge", I'm not talking about
> the explicit software IRQ ACK, I'm talking about the hardware
> protocol, between the PIC and the CPU), the PIC will then report irq 7
> as the interrupt - regardless of what the original was.
> 
> The reason is almost always something like
> 
>  - CPU interrupts are disabled or masked
> 
>  - driver does a write to the external hardware that causes an
> interrupt to be raised

Which should be a non issue because _ALL_ PIC irq lines are masked at the
PIC itself. All interrupts are routed through IOAPIC. So unless the IOAPIC
sports similar behaviour the PIC should not ever observe that scenario.

But, because the silly firmware comes out of suspend with all PIC lines
unmasked for whatever reason, the PIC can observe that IRQ being raised and
the CPU not handling it. So yes, I forgot about 7 being magic, but I still
think it's the firmware which causes it by unmasking the PIC irqs.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner  wrote:
>
> So the old scheme silently consumed the spurious vector. I added debug code
> to that effect to 4.14 and on that machine IRQ7 is triggered at the same
> point post resume and the core code drops it silently because the interrupt
> is marked masked and no action assigned.
>
> So the only difference to today is that the new code complains, while the
> old one does an extra mask of the already masked IOAPIC pin and silently
> returns.

Great debugging, and it looks like Rafael has a patch that already got
positive testing.

I just wanted to pipe up about that "irq7", because judging from your
email it seems like you think it's a real irq:

>Now there is a race
> whether the kernel resume path manages to mask the PIC again early enough
> before something triggers IRQ7 or not.

..and that's not how the PIC works.

In fact, "legacy irq 7" is the _normal_ and very traditional spurious
interrupt, and it's documented. If the PIC gets an interrupt from
_any_ source, but the interrupt goes away before the PIC gets an
acknowledge from the CPU (and by "acknowledge", I'm not talking about
the explicit software IRQ ACK, I'm talking about the hardware
protocol, between the PIC and the CPU), the PIC will then report irq 7
as the interrupt - regardless of what the original was.

The reason is almost always something like

 - CPU interrupts are disabled or masked

 - driver does a write to the external hardware that causes an
interrupt to be raised

 - CPU doesn't react to the irq due to the disabled/masked nature

 - but the driver then does something that masks the interrupt again

 - interrupts are enabled/unmasked on the CPU

 - CPU now acks the interrupt, but the PIC no longer sees any
interrupt source, so the PIC (that has to reply with *something*)
replies with that documented spurious irq7.

To confuse things further, irq7 is not _exclusively_ the spurious
interrupt, You can definitely put real hardware and connect it to pin7
of the PIC, and get real irq7 reports.

And to confuse things even *more*, this "irq7" thing is per-PIC, and
the PC model obviously has the whole "nested PIC" thing where the
second PIC is connected to irq2 of the first PIC. So there are *two*
different "spurious interrupt" reports, one for each PIC.

Anyway, to avoid this issue, drivers should strive to

 (a) actually take the interrupt when doing things that can cause
them, and have the interrupt handler do whatever it is that causes the
interrupt to go away (ie: "normal operation")

 (b) if you play games with clearing the source of the interrupt
_without_ taking the interrupt, you should strive to basically mask
the interrupt first.

So to do (b) you can do something like

  mask_device_interrupt(dev);
  read_from_device_to_synchronize(dev);

instead of (or perhaps _before_) disabling interrupts at a CPU level.
Suspend/resume obviously does tend to play games with these kinds of
things where you are no longer in "normal operation" and you do setup
without having interrupts actually enabled.

Or you can just decide that spurious interrupts are ok, and ignore the
issue. But they *can* be very confusing, and obviously in this case
that confusion then seems to have caused actual problems.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner  wrote:
>
> So the old scheme silently consumed the spurious vector. I added debug code
> to that effect to 4.14 and on that machine IRQ7 is triggered at the same
> point post resume and the core code drops it silently because the interrupt
> is marked masked and no action assigned.
>
> So the only difference to today is that the new code complains, while the
> old one does an extra mask of the already masked IOAPIC pin and silently
> returns.

Great debugging, and it looks like Rafael has a patch that already got
positive testing.

I just wanted to pipe up about that "irq7", because judging from your
email it seems like you think it's a real irq:

>Now there is a race
> whether the kernel resume path manages to mask the PIC again early enough
> before something triggers IRQ7 or not.

..and that's not how the PIC works.

In fact, "legacy irq 7" is the _normal_ and very traditional spurious
interrupt, and it's documented. If the PIC gets an interrupt from
_any_ source, but the interrupt goes away before the PIC gets an
acknowledge from the CPU (and by "acknowledge", I'm not talking about
the explicit software IRQ ACK, I'm talking about the hardware
protocol, between the PIC and the CPU), the PIC will then report irq 7
as the interrupt - regardless of what the original was.

The reason is almost always something like

 - CPU interrupts are disabled or masked

 - driver does a write to the external hardware that causes an
interrupt to be raised

 - CPU doesn't react to the irq due to the disabled/masked nature

 - but the driver then does something that masks the interrupt again

 - interrupts are enabled/unmasked on the CPU

 - CPU now acks the interrupt, but the PIC no longer sees any
interrupt source, so the PIC (that has to reply with *something*)
replies with that documented spurious irq7.

To confuse things further, irq7 is not _exclusively_ the spurious
interrupt, You can definitely put real hardware and connect it to pin7
of the PIC, and get real irq7 reports.

And to confuse things even *more*, this "irq7" thing is per-PIC, and
the PC model obviously has the whole "nested PIC" thing where the
second PIC is connected to irq2 of the first PIC. So there are *two*
different "spurious interrupt" reports, one for each PIC.

Anyway, to avoid this issue, drivers should strive to

 (a) actually take the interrupt when doing things that can cause
them, and have the interrupt handler do whatever it is that causes the
interrupt to go away (ie: "normal operation")

 (b) if you play games with clearing the source of the interrupt
_without_ taking the interrupt, you should strive to basically mask
the interrupt first.

So to do (b) you can do something like

  mask_device_interrupt(dev);
  read_from_device_to_synchronize(dev);

instead of (or perhaps _before_) disabling interrupts at a CPU level.
Suspend/resume obviously does tend to play games with these kinds of
things where you are no longer in "normal operation" and you do setup
without having interrupts actually enabled.

Or you can just decide that spurious interrupts are ok, and ignore the
issue. But they *can* be very confusing, and obviously in this case
that confusion then seems to have caused actual problems.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Maarten Lankhorst]

Op 14-12-17 om 16:54 schreef Rafael J. Wysocki:
> On Thursday, December 14, 2017 4:52:22 PM CET Thomas Gleixner wrote:
>> On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
>>> The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
>>> in fact requires the device to be in D0, so the caller should put it into
>>> D0 instead of trying to "update" its power state.
>>>
>>> [Note that the PCI layer doesn't put devices into low-power states during 
>>> the
>>> hibernation's "freeze" transition, but drivers can legitimately do that in
>>> their "freeze" callbacks which was overlooked in that code and that's what
>>> i915 does.]
>>>
>>> So IMO what we need is the change below.  I'm going to test it shortly,
>>> but please give it a go too.
>> So now this looks more reasonable:
>>
>>   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>>   __pci_write_msi_msg: Not written
>>   ...
>>   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
>> [thaw]
>>   pci_pm_thaw_noirq <-dpm_run_callback
>>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>>   device_pm_callback_end: i915 :00:02.0, err=0
>>   ...
>>   resume_irqs: Resume 125
>>   ...
>>   irq_handler_entry: irq=125 name=i915
> Cool.
>
> Let me respin it with a changelog etc then.
>
> Thanks,
> Rafael
>
>
The machine I was using for reproducing the bug appears to be fixed with this 
patch, so I now sent
it to intel's trybot for results.

https://patchwork.freedesktop.org/series/35367/

Thanks for looking at the bug!

~Maarten

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Maarten Lankhorst]

Op 14-12-17 om 16:54 schreef Rafael J. Wysocki:
> On Thursday, December 14, 2017 4:52:22 PM CET Thomas Gleixner wrote:
>> On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
>>> The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
>>> in fact requires the device to be in D0, so the caller should put it into
>>> D0 instead of trying to "update" its power state.
>>>
>>> [Note that the PCI layer doesn't put devices into low-power states during 
>>> the
>>> hibernation's "freeze" transition, but drivers can legitimately do that in
>>> their "freeze" callbacks which was overlooked in that code and that's what
>>> i915 does.]
>>>
>>> So IMO what we need is the change below.  I'm going to test it shortly,
>>> but please give it a go too.
>> So now this looks more reasonable:
>>
>>   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>>   __pci_write_msi_msg: Not written
>>   ...
>>   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
>> [thaw]
>>   pci_pm_thaw_noirq <-dpm_run_callback
>>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>>   device_pm_callback_end: i915 :00:02.0, err=0
>>   ...
>>   resume_irqs: Resume 125
>>   ...
>>   irq_handler_entry: irq=125 name=i915
> Cool.
>
> Let me respin it with a changelog etc then.
>
> Thanks,
> Rafael
>
>
The machine I was using for reproducing the bug appears to be fixed with this 
patch, so I now sent
it to intel's trybot for results.

https://patchwork.freedesktop.org/series/35367/

Thanks for looking at the bug!

~Maarten

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thursday, December 14, 2017 4:52:22 PM CET Thomas Gleixner wrote:
> On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> > The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
> > in fact requires the device to be in D0, so the caller should put it into
> > D0 instead of trying to "update" its power state.
> > 
> > [Note that the PCI layer doesn't put devices into low-power states during 
> > the
> > hibernation's "freeze" transition, but drivers can legitimately do that in
> > their "freeze" callbacks which was overlooked in that code and that's what
> > i915 does.]
> > 
> > So IMO what we need is the change below.  I'm going to test it shortly,
> > but please give it a go too.
> 
> So now this looks more reasonable:
> 
>   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>   __pci_write_msi_msg: Not written
>   ...
>   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
> [thaw]
>   pci_pm_thaw_noirq <-dpm_run_callback
>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>   device_pm_callback_end: i915 :00:02.0, err=0
>   ...
>   resume_irqs: Resume 125
>   ...
>   irq_handler_entry: irq=125 name=i915

Cool.

Let me respin it with a changelog etc then.

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thursday, December 14, 2017 4:52:22 PM CET Thomas Gleixner wrote:
> On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> > The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
> > in fact requires the device to be in D0, so the caller should put it into
> > D0 instead of trying to "update" its power state.
> > 
> > [Note that the PCI layer doesn't put devices into low-power states during 
> > the
> > hibernation's "freeze" transition, but drivers can legitimately do that in
> > their "freeze" callbacks which was overlooked in that code and that's what
> > i915 does.]
> > 
> > So IMO what we need is the change below.  I'm going to test it shortly,
> > but please give it a go too.
> 
> So now this looks more reasonable:
> 
>   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>   __pci_write_msi_msg: Not written
>   ...
>   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
> [thaw]
>   pci_pm_thaw_noirq <-dpm_run_callback
>   __pci_write_msi_msg: :00:02.0 fee0100c 412a 
>   device_pm_callback_end: i915 :00:02.0, err=0
>   ...
>   resume_irqs: Resume 125
>   ...
>   irq_handler_entry: irq=125 name=i915

Cool.

Let me respin it with a changelog etc then.

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
> in fact requires the device to be in D0, so the caller should put it into
> D0 instead of trying to "update" its power state.
> 
> [Note that the PCI layer doesn't put devices into low-power states during the
> hibernation's "freeze" transition, but drivers can legitimately do that in
> their "freeze" callbacks which was overlooked in that code and that's what
> i915 does.]
> 
> So IMO what we need is the change below.  I'm going to test it shortly,
> but please give it a go too.

So now this looks more reasonable:

  irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
  __pci_write_msi_msg: :00:02.0 fee0100c 412a 
  __pci_write_msi_msg: Not written
  ...
  device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
[thaw]
  pci_pm_thaw_noirq <-dpm_run_callback
  __pci_write_msi_msg: :00:02.0 fee0100c 412a 
  device_pm_callback_end: i915 :00:02.0, err=0
  ...
  resume_irqs: Resume 125
  ...
  irq_handler_entry: irq=125 name=i915

Thanks,

tglx

> ---
>  drivers/pci/pci-driver.c |7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> Index: linux-pm/drivers/pci/pci-driver.c
> ===
> --- linux-pm.orig/drivers/pci/pci-driver.c
> +++ linux-pm/drivers/pci/pci-driver.c
> @@ -1027,7 +1027,12 @@ static int pci_pm_thaw_noirq(struct devi
>   if (pci_has_legacy_pm_support(pci_dev))
>   return pci_legacy_resume_early(dev);
>  
> - pci_update_current_state(pci_dev, PCI_D0);
> + /*
> +  * pci_restore_state() requires the device to be in D0 (because of MSI
> +  * restoration among other things), so force it into D0 in case the
> +  * driver's "freeze" callbacks put it into a low-power state directly.
> +  */
> + pci_set_power_state(pci_dev, PCI_D0);
>   pci_restore_state(pci_dev);
>  
>   if (drv && drv->pm && drv->pm->thaw_noirq)
> 
> 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
> in fact requires the device to be in D0, so the caller should put it into
> D0 instead of trying to "update" its power state.
> 
> [Note that the PCI layer doesn't put devices into low-power states during the
> hibernation's "freeze" transition, but drivers can legitimately do that in
> their "freeze" callbacks which was overlooked in that code and that's what
> i915 does.]
> 
> So IMO what we need is the change below.  I'm going to test it shortly,
> but please give it a go too.

So now this looks more reasonable:

  irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
  __pci_write_msi_msg: :00:02.0 fee0100c 412a 
  __pci_write_msi_msg: Not written
  ...
  device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
[thaw]
  pci_pm_thaw_noirq <-dpm_run_callback
  __pci_write_msi_msg: :00:02.0 fee0100c 412a 
  device_pm_callback_end: i915 :00:02.0, err=0
  ...
  resume_irqs: Resume 125
  ...
  irq_handler_entry: irq=125 name=i915

Thanks,

tglx

> ---
>  drivers/pci/pci-driver.c |7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> Index: linux-pm/drivers/pci/pci-driver.c
> ===
> --- linux-pm.orig/drivers/pci/pci-driver.c
> +++ linux-pm/drivers/pci/pci-driver.c
> @@ -1027,7 +1027,12 @@ static int pci_pm_thaw_noirq(struct devi
>   if (pci_has_legacy_pm_support(pci_dev))
>   return pci_legacy_resume_early(dev);
>  
> - pci_update_current_state(pci_dev, PCI_D0);
> + /*
> +  * pci_restore_state() requires the device to be in D0 (because of MSI
> +  * restoration among other things), so force it into D0 in case the
> +  * driver's "freeze" callbacks put it into a low-power state directly.
> +  */
> + pci_set_power_state(pci_dev, PCI_D0);
>   pci_restore_state(pci_dev);
>  
>   if (drv && drv->pm && drv->pm->thaw_noirq)
> 
> 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thursday, December 14, 2017 1:30:37 PM CET Thomas Gleixner wrote:
> On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> > On Thursday, December 14, 2017 12:54:05 PM CET Thomas Gleixner wrote:
> > > Now the graphics issue is a different story. That only happens on
> > > hibernation after doing the snapshot. There all non boot cpus are onlined
> > > again and after that the devices are 'thawed'. The following reenable of
> > > interrupts fails because i915 is not in PCI_D0 state.
> > > 
> > > Suspend:
> > > 
> > >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> > >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> > >
> > >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > > bus [resume]
> > >pci_pm_resume_noirq <-dpm_run_callback
> > >pci_pm_resume_noirq <-dpm_run_callback
> > >pci_pm_default_resume_early <-pci_pm_resume_noirq
> > >pci_pm_default_resume_early <-pci_pm_resume_noirq
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set 
> > > the new affinity
> > >device_pm_callback_end: i915 :00:02.0, err=0
> > 
> > So this works, because we power up the device during resume even if it
> > had been suspended (via runtime PM) before the suspend started.
> > 
> > > Hibernate:
> > > 
> > >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> > >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> > >
> > >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > > bus [thaw]
> > >pci_pm_thaw_noirq <-dpm_run_callback
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> > >__pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
> > >device_pm_callback_end: i915 :00:02.0, err=0
> > 
> > And here we try to leave the device alone which is OK for devices in D0,
> > but not for suspended ones.
> > 
> > It looks like we need to power up them at the "thaw" time too or at least
> > I don't see how to address that differently.
> 
> The question is whether the code which brings the device out of D0 should
> write the message unconditionally. That would be sufficient I think.

It doesn't have to do that.

The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
in fact requires the device to be in D0, so the caller should put it into
D0 instead of trying to "update" its power state.

[Note that the PCI layer doesn't put devices into low-power states during the
hibernation's "freeze" transition, but drivers can legitimately do that in
their "freeze" callbacks which was overlooked in that code and that's what
i915 does.]

So IMO what we need is the change below.  I'm going to test it shortly,
but please give it a go too.

---
 drivers/pci/pci-driver.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/pci/pci-driver.c
===
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -1027,7 +1027,12 @@ static int pci_pm_thaw_noirq(struct devi
if (pci_has_legacy_pm_support(pci_dev))
return pci_legacy_resume_early(dev);
 
-   pci_update_current_state(pci_dev, PCI_D0);
+   /*
+* pci_restore_state() requires the device to be in D0 (because of MSI
+* restoration among other things), so force it into D0 in case the
+* driver's "freeze" callbacks put it into a low-power state directly.
+*/
+   pci_set_power_state(pci_dev, PCI_D0);
pci_restore_state(pci_dev);
 
if (drv && drv->pm && drv->pm->thaw_noirq)

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thursday, December 14, 2017 1:30:37 PM CET Thomas Gleixner wrote:
> On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> > On Thursday, December 14, 2017 12:54:05 PM CET Thomas Gleixner wrote:
> > > Now the graphics issue is a different story. That only happens on
> > > hibernation after doing the snapshot. There all non boot cpus are onlined
> > > again and after that the devices are 'thawed'. The following reenable of
> > > interrupts fails because i915 is not in PCI_D0 state.
> > > 
> > > Suspend:
> > > 
> > >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> > >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> > >
> > >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > > bus [resume]
> > >pci_pm_resume_noirq <-dpm_run_callback
> > >pci_pm_resume_noirq <-dpm_run_callback
> > >pci_pm_default_resume_early <-pci_pm_resume_noirq
> > >pci_pm_default_resume_early <-pci_pm_resume_noirq
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set 
> > > the new affinity
> > >device_pm_callback_end: i915 :00:02.0, err=0
> > 
> > So this works, because we power up the device during resume even if it
> > had been suspended (via runtime PM) before the suspend started.
> > 
> > > Hibernate:
> > > 
> > >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> > >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> > >
> > >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > > bus [thaw]
> > >pci_pm_thaw_noirq <-dpm_run_callback
> > >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> > >__pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
> > >device_pm_callback_end: i915 :00:02.0, err=0
> > 
> > And here we try to leave the device alone which is OK for devices in D0,
> > but not for suspended ones.
> > 
> > It looks like we need to power up them at the "thaw" time too or at least
> > I don't see how to address that differently.
> 
> The question is whether the code which brings the device out of D0 should
> write the message unconditionally. That would be sufficient I think.

It doesn't have to do that.

The problem here is that pci_pm_thaw_noirq() calls pci_restore_state() which
in fact requires the device to be in D0, so the caller should put it into
D0 instead of trying to "update" its power state.

[Note that the PCI layer doesn't put devices into low-power states during the
hibernation's "freeze" transition, but drivers can legitimately do that in
their "freeze" callbacks which was overlooked in that code and that's what
i915 does.]

So IMO what we need is the change below.  I'm going to test it shortly,
but please give it a go too.

---
 drivers/pci/pci-driver.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/pci/pci-driver.c
===
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -1027,7 +1027,12 @@ static int pci_pm_thaw_noirq(struct devi
if (pci_has_legacy_pm_support(pci_dev))
return pci_legacy_resume_early(dev);
 
-   pci_update_current_state(pci_dev, PCI_D0);
+   /*
+* pci_restore_state() requires the device to be in D0 (because of MSI
+* restoration among other things), so force it into D0 in case the
+* driver's "freeze" callbacks put it into a low-power state directly.
+*/
+   pci_set_power_state(pci_dev, PCI_D0);
pci_restore_state(pci_dev);
 
if (drv && drv->pm && drv->pm->thaw_noirq)

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Thomas Gleixner wrote:
> Now, what's different vs. 4.14:
> 
> The 4.14 code accidentaly had the irq descriptor for this vector still
> populated in the old CPU due to the convoluted way the vector allocation
> worked. I have still to investigate if one of those cases is actually
> leaking the descriptor, which would be a fatal bug.

It doesn't leak. It repopulates it at the same place out of sheer luck.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Thomas Gleixner wrote:
> Now, what's different vs. 4.14:
> 
> The 4.14 code accidentaly had the irq descriptor for this vector still
> populated in the old CPU due to the convoluted way the vector allocation
> worked. I have still to investigate if one of those cases is actually
> leaking the descriptor, which would be a fatal bug.

It doesn't leak. It repopulates it at the same place out of sheer luck.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> On Thursday, December 14, 2017 12:54:05 PM CET Thomas Gleixner wrote:
> > Now the graphics issue is a different story. That only happens on
> > hibernation after doing the snapshot. There all non boot cpus are onlined
> > again and after that the devices are 'thawed'. The following reenable of
> > interrupts fails because i915 is not in PCI_D0 state.
> > 
> > Suspend:
> > 
> >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> >
> >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > bus [resume]
> >pci_pm_resume_noirq <-dpm_run_callback
> >pci_pm_resume_noirq <-dpm_run_callback
> >pci_pm_default_resume_early <-pci_pm_resume_noirq
> >pci_pm_default_resume_early <-pci_pm_resume_noirq
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set the 
> > new affinity
> >device_pm_callback_end: i915 :00:02.0, err=0
> 
> So this works, because we power up the device during resume even if it
> had been suspended (via runtime PM) before the suspend started.
> 
> > Hibernate:
> > 
> >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> >
> >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > bus [thaw]
> >pci_pm_thaw_noirq <-dpm_run_callback
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> >__pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
> >device_pm_callback_end: i915 :00:02.0, err=0
> 
> And here we try to leave the device alone which is OK for devices in D0,
> but not for suspended ones.
> 
> It looks like we need to power up them at the "thaw" time too or at least
> I don't see how to address that differently.

The question is whether the code which brings the device out of D0 should
write the message unconditionally. That would be sufficient I think.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 14 Dec 2017, Rafael J. Wysocki wrote:
> On Thursday, December 14, 2017 12:54:05 PM CET Thomas Gleixner wrote:
> > Now the graphics issue is a different story. That only happens on
> > hibernation after doing the snapshot. There all non boot cpus are onlined
> > again and after that the devices are 'thawed'. The following reenable of
> > interrupts fails because i915 is not in PCI_D0 state.
> > 
> > Suspend:
> > 
> >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> >
> >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > bus [resume]
> >pci_pm_resume_noirq <-dpm_run_callback
> >pci_pm_resume_noirq <-dpm_run_callback
> >pci_pm_default_resume_early <-pci_pm_resume_noirq
> >pci_pm_default_resume_early <-pci_pm_resume_noirq
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set the 
> > new affinity
> >device_pm_callback_end: i915 :00:02.0, err=0
> 
> So this works, because we power up the device during resume even if it
> had been suspended (via runtime PM) before the suspend started.
> 
> > Hibernate:
> > 
> >irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> >__pci_write_msi_msg: Not written <- Device not in PCI_D0
> >
> >device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq 
> > bus [thaw]
> >pci_pm_thaw_noirq <-dpm_run_callback
> >__pci_write_msi_msg: :00:02.0 fee0100c 412a
> >__pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
> >device_pm_callback_end: i915 :00:02.0, err=0
> 
> And here we try to leave the device alone which is OK for devices in D0,
> but not for suspended ones.
> 
> It looks like we need to power up them at the "thaw" time too or at least
> I don't see how to address that differently.

The question is whether the code which brings the device out of D0 should
write the message unconditionally. That would be sufficient I think.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thursday, December 14, 2017 12:54:05 PM CET Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > > > 
> > > > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > > > wrote:
> > > > > >
> > > > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > > > figure
> > > > > > out what now causes that spurious interrupt to surface out of the 
> > > > > > blue.
> > > > > 
> > > > > Perhaps just timing?
> > > > 
> > > > That's what I'm trying to figure out right now, because that is the only
> > > > sensible explanation left. The whole machinery of suspend is exactly the
> > > > same with and without the vector changes. I instrumented all functions
> > > > involved and the picture is the same. I even do not see any fundamental
> > > > timing differences where one would say: That's it.
> > > > 
> > > > What puzzles me even more is that in the range of commits I'm fiddling 
> > > > with
> > > > there is no other change than the vector management stuff and the point
> > > > where it breaks makes no sense at all. The point Maarten bisected it to
> > > > works nicely here, so that might just point to a very subtle timing 
> > > > issue.
> > > 
> > > After doing more debugging on this it turns out that this looks like a
> > > legacy interrupt coming in. The vector number is always 55, which is 
> > > legacy
> > > IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> > > masked and vector 55 is completely unused.
> > > 
> > > More questions than answers. Still investigating.
> 
> At least that one could be explained by the changes. In the previous
> management scheme the IOAPIC interrupts were always allocated even when the
> interrupt was not in use. The new scheme does not longer do that because
> people complained about the vector waste (16 vectors on each CPU) and it
> got rid of all the special casing of IRQ0-15.
> 
> So the old scheme silently consumed the spurious vector. I added debug code
> to that effect to 4.14 and on that machine IRQ7 is triggered at the same
> point post resume and the core code drops it silently because the interrupt
> is marked masked and no action assigned.
> 
> So the only difference to today is that the new code complains, while the
> old one does an extra mask of the already masked IOAPIC pin and silently
> returns.
> 
> After quite some investigation I found out that its independent of the
> graphics thing. That's a genuine issue on that platform which seems to emit
> random legacy vectors which were never ever used for unknown reasons. I
> verified that both the IOAPIC and the PIC are masked, so they cannot send
> crap. Though it turned out that the silly firmware unmasks the PIC and
> leaves it that way when it returns from suspend. Now there is a race
> whether the kernel resume path manages to mask the PIC again early enough
> before something triggers IRQ7 or not. Adding/removing debug code makes the
> problem come and go. So I really don't worry about that one and rather
> prefer to have the spurious interrupt printed than silently consumed by
> chance.

OK

> Now the graphics issue is a different story. That only happens on
> hibernation after doing the snapshot. There all non boot cpus are onlined
> again and after that the devices are 'thawed'. The following reenable of
> interrupts fails because i915 is not in PCI_D0 state.
> 
> Suspend:
> 
>irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>__pci_write_msi_msg: :00:02.0 fee0100c 412a
>__pci_write_msi_msg: Not written <- Device not in PCI_D0
>
>device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
> [resume]
>pci_pm_resume_noirq <-dpm_run_callback
>pci_pm_resume_noirq <-dpm_run_callback
>pci_pm_default_resume_early <-pci_pm_resume_noirq
>pci_pm_default_resume_early <-pci_pm_resume_noirq
>__pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set the 
> new affinity
>device_pm_callback_end: i915 :00:02.0, err=0

So this works, because we power up the device during resume even if it
had been suspended (via runtime PM) before the suspend started.

> Hibernate:
> 
>irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>__pci_write_msi_msg: :00:02.0 fee0100c 412a
>__pci_write_msi_msg: Not written <- Device not in PCI_D0
>
>device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
> [thaw]
>pci_pm_thaw_noirq <-dpm_run_callback
>__pci_write_msi_msg: :00:02.0 fee0100c 412a
>__pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
>device_pm_callback_end: i915 :00:02.0, err=0

And here we try to leave the device alone which is OK for devices in D0,
but not for suspended 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Thursday, December 14, 2017 12:54:05 PM CET Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > > > 
> > > > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > > > wrote:
> > > > > >
> > > > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > > > figure
> > > > > > out what now causes that spurious interrupt to surface out of the 
> > > > > > blue.
> > > > > 
> > > > > Perhaps just timing?
> > > > 
> > > > That's what I'm trying to figure out right now, because that is the only
> > > > sensible explanation left. The whole machinery of suspend is exactly the
> > > > same with and without the vector changes. I instrumented all functions
> > > > involved and the picture is the same. I even do not see any fundamental
> > > > timing differences where one would say: That's it.
> > > > 
> > > > What puzzles me even more is that in the range of commits I'm fiddling 
> > > > with
> > > > there is no other change than the vector management stuff and the point
> > > > where it breaks makes no sense at all. The point Maarten bisected it to
> > > > works nicely here, so that might just point to a very subtle timing 
> > > > issue.
> > > 
> > > After doing more debugging on this it turns out that this looks like a
> > > legacy interrupt coming in. The vector number is always 55, which is 
> > > legacy
> > > IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> > > masked and vector 55 is completely unused.
> > > 
> > > More questions than answers. Still investigating.
> 
> At least that one could be explained by the changes. In the previous
> management scheme the IOAPIC interrupts were always allocated even when the
> interrupt was not in use. The new scheme does not longer do that because
> people complained about the vector waste (16 vectors on each CPU) and it
> got rid of all the special casing of IRQ0-15.
> 
> So the old scheme silently consumed the spurious vector. I added debug code
> to that effect to 4.14 and on that machine IRQ7 is triggered at the same
> point post resume and the core code drops it silently because the interrupt
> is marked masked and no action assigned.
> 
> So the only difference to today is that the new code complains, while the
> old one does an extra mask of the already masked IOAPIC pin and silently
> returns.
> 
> After quite some investigation I found out that its independent of the
> graphics thing. That's a genuine issue on that platform which seems to emit
> random legacy vectors which were never ever used for unknown reasons. I
> verified that both the IOAPIC and the PIC are masked, so they cannot send
> crap. Though it turned out that the silly firmware unmasks the PIC and
> leaves it that way when it returns from suspend. Now there is a race
> whether the kernel resume path manages to mask the PIC again early enough
> before something triggers IRQ7 or not. Adding/removing debug code makes the
> problem come and go. So I really don't worry about that one and rather
> prefer to have the spurious interrupt printed than silently consumed by
> chance.

OK

> Now the graphics issue is a different story. That only happens on
> hibernation after doing the snapshot. There all non boot cpus are onlined
> again and after that the devices are 'thawed'. The following reenable of
> interrupts fails because i915 is not in PCI_D0 state.
> 
> Suspend:
> 
>irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>__pci_write_msi_msg: :00:02.0 fee0100c 412a
>__pci_write_msi_msg: Not written <- Device not in PCI_D0
>
>device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
> [resume]
>pci_pm_resume_noirq <-dpm_run_callback
>pci_pm_resume_noirq <-dpm_run_callback
>pci_pm_default_resume_early <-pci_pm_resume_noirq
>pci_pm_default_resume_early <-pci_pm_resume_noirq
>__pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set the 
> new affinity
>device_pm_callback_end: i915 :00:02.0, err=0

So this works, because we power up the device during resume even if it
had been suspended (via runtime PM) before the suspend started.

> Hibernate:
> 
>irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
>__pci_write_msi_msg: :00:02.0 fee0100c 412a
>__pci_write_msi_msg: Not written <- Device not in PCI_D0
>
>device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
> [thaw]
>pci_pm_thaw_noirq <-dpm_run_callback
>__pci_write_msi_msg: :00:02.0 fee0100c 412a
>__pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
>device_pm_callback_end: i915 :00:02.0, err=0

And here we try to leave the device alone which is OK for devices in D0,
but not for suspended ones.

It looks like we 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > > 
> > > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > > wrote:
> > > > >
> > > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > > figure
> > > > > out what now causes that spurious interrupt to surface out of the 
> > > > > blue.
> > > > 
> > > > Perhaps just timing?
> > > 
> > > That's what I'm trying to figure out right now, because that is the only
> > > sensible explanation left. The whole machinery of suspend is exactly the
> > > same with and without the vector changes. I instrumented all functions
> > > involved and the picture is the same. I even do not see any fundamental
> > > timing differences where one would say: That's it.
> > > 
> > > What puzzles me even more is that in the range of commits I'm fiddling 
> > > with
> > > there is no other change than the vector management stuff and the point
> > > where it breaks makes no sense at all. The point Maarten bisected it to
> > > works nicely here, so that might just point to a very subtle timing issue.
> > 
> > After doing more debugging on this it turns out that this looks like a
> > legacy interrupt coming in. The vector number is always 55, which is legacy
> > IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> > masked and vector 55 is completely unused.
> > 
> > More questions than answers. Still investigating.

At least that one could be explained by the changes. In the previous
management scheme the IOAPIC interrupts were always allocated even when the
interrupt was not in use. The new scheme does not longer do that because
people complained about the vector waste (16 vectors on each CPU) and it
got rid of all the special casing of IRQ0-15.

So the old scheme silently consumed the spurious vector. I added debug code
to that effect to 4.14 and on that machine IRQ7 is triggered at the same
point post resume and the core code drops it silently because the interrupt
is marked masked and no action assigned.

So the only difference to today is that the new code complains, while the
old one does an extra mask of the already masked IOAPIC pin and silently
returns.

After quite some investigation I found out that its independent of the
graphics thing. That's a genuine issue on that platform which seems to emit
random legacy vectors which were never ever used for unknown reasons. I
verified that both the IOAPIC and the PIC are masked, so they cannot send
crap. Though it turned out that the silly firmware unmasks the PIC and
leaves it that way when it returns from suspend. Now there is a race
whether the kernel resume path manages to mask the PIC again early enough
before something triggers IRQ7 or not. Adding/removing debug code makes the
problem come and go. So I really don't worry about that one and rather
prefer to have the spurious interrupt printed than silently consumed by
chance.

Now the graphics issue is a different story. That only happens on
hibernation after doing the snapshot. There all non boot cpus are onlined
again and after that the devices are 'thawed'. The following reenable of
interrupts fails because i915 is not in PCI_D0 state.

Suspend:

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: :00:02.0 fee0100c 412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   
   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
[resume]
   pci_pm_resume_noirq <-dpm_run_callback
   pci_pm_resume_noirq <-dpm_run_callback
   pci_pm_default_resume_early <-pci_pm_resume_noirq
   pci_pm_default_resume_early <-pci_pm_resume_noirq
   __pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set the new 
affinity
   device_pm_callback_end: i915 :00:02.0, err=0

Hibernate:

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: :00:02.0 fee0100c 412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   
   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
[thaw]
   pci_pm_thaw_noirq <-dpm_run_callback
   __pci_write_msi_msg: :00:02.0 fee0100c 412a
   __pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
   device_pm_callback_end: i915 :00:02.0, err=0

So that code path fails to set the new affinity because at the point where
the MSI msg should be written the device state is != PCI_D0.

Now, what's different vs. 4.14:

The 4.14 code accidentaly had the irq descriptor for this vector still
populated in the old CPU due to the convoluted way the vector allocation
worked. I have still to investigate if one of those cases is actually
leaking the descriptor, which would be a fatal bug.

But the new code does a proper cleanup and does not repopulate it on 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > > 
> > > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > > wrote:
> > > > >
> > > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > > figure
> > > > > out what now causes that spurious interrupt to surface out of the 
> > > > > blue.
> > > > 
> > > > Perhaps just timing?
> > > 
> > > That's what I'm trying to figure out right now, because that is the only
> > > sensible explanation left. The whole machinery of suspend is exactly the
> > > same with and without the vector changes. I instrumented all functions
> > > involved and the picture is the same. I even do not see any fundamental
> > > timing differences where one would say: That's it.
> > > 
> > > What puzzles me even more is that in the range of commits I'm fiddling 
> > > with
> > > there is no other change than the vector management stuff and the point
> > > where it breaks makes no sense at all. The point Maarten bisected it to
> > > works nicely here, so that might just point to a very subtle timing issue.
> > 
> > After doing more debugging on this it turns out that this looks like a
> > legacy interrupt coming in. The vector number is always 55, which is legacy
> > IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> > masked and vector 55 is completely unused.
> > 
> > More questions than answers. Still investigating.

At least that one could be explained by the changes. In the previous
management scheme the IOAPIC interrupts were always allocated even when the
interrupt was not in use. The new scheme does not longer do that because
people complained about the vector waste (16 vectors on each CPU) and it
got rid of all the special casing of IRQ0-15.

So the old scheme silently consumed the spurious vector. I added debug code
to that effect to 4.14 and on that machine IRQ7 is triggered at the same
point post resume and the core code drops it silently because the interrupt
is marked masked and no action assigned.

So the only difference to today is that the new code complains, while the
old one does an extra mask of the already masked IOAPIC pin and silently
returns.

After quite some investigation I found out that its independent of the
graphics thing. That's a genuine issue on that platform which seems to emit
random legacy vectors which were never ever used for unknown reasons. I
verified that both the IOAPIC and the PIC are masked, so they cannot send
crap. Though it turned out that the silly firmware unmasks the PIC and
leaves it that way when it returns from suspend. Now there is a race
whether the kernel resume path manages to mask the PIC again early enough
before something triggers IRQ7 or not. Adding/removing debug code makes the
problem come and go. So I really don't worry about that one and rather
prefer to have the spurious interrupt printed than silently consumed by
chance.

Now the graphics issue is a different story. That only happens on
hibernation after doing the snapshot. There all non boot cpus are onlined
again and after that the devices are 'thawed'. The following reenable of
interrupts fails because i915 is not in PCI_D0 state.

Suspend:

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: :00:02.0 fee0100c 412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   
   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
[resume]
   pci_pm_resume_noirq <-dpm_run_callback
   pci_pm_resume_noirq <-dpm_run_callback
   pci_pm_default_resume_early <-pci_pm_resume_noirq
   pci_pm_default_resume_early <-pci_pm_resume_noirq
   __pci_write_msi_msg: :00:02.0 fee0100c 412a  <-- Set the new 
affinity
   device_pm_callback_end: i915 :00:02.0, err=0

Hibernate:

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: :00:02.0 fee0100c 412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   
   device_pm_callback_start: i915 :00:02.0, parent: pci:00, noirq bus 
[thaw]
   pci_pm_thaw_noirq <-dpm_run_callback
   __pci_write_msi_msg: :00:02.0 fee0100c 412a
   __pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
   device_pm_callback_end: i915 :00:02.0, err=0

So that code path fails to set the new affinity because at the point where
the MSI msg should be written the device state is != PCI_D0.

Now, what's different vs. 4.14:

The 4.14 code accidentaly had the irq descriptor for this vector still
populated in the old CPU due to the convoluted way the vector allocation
worked. I have still to investigate if one of those cases is actually
leaking the descriptor, which would be a fatal bug.

But the new code does a proper cleanup and does not repopulate it on the
offline CPU. So 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Wed, Dec 13, 2017 at 11:39 PM, Rafael J. Wysocki  wrote:
> On Wednesday, December 13, 2017 7:19:17 PM CET Thomas Gleixner wrote:
>> On Wed, 13 Dec 2017, Linus Torvalds wrote:
>>
>> > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
>> > wrote:
>> > >
>> > > Definitely. That was fragile forever but puzzles me is that I can't 
>> > > figure
>> > > out what now causes that spurious interrupt to surface out of the blue.
>> >
>> > Perhaps just timing?
>>
>> That's what I'm trying to figure out right now, because that is the only
>> sensible explanation left. The whole machinery of suspend is exactly the
>> same with and without the vector changes. I instrumented all functions
>> involved and the picture is the same. I even do not see any fundamental
>> timing differences where one would say: That's it.
>>
>> What puzzles me even more is that in the range of commits I'm fiddling with
>> there is no other change than the vector management stuff and the point
>> where it breaks makes no sense at all. The point Maarten bisected it to
>> works nicely here, so that might just point to a very subtle timing issue.
>>
>> > How hard would it be to change the ordering to just redirect irqs first?
>>
>> The whole interrupt redirection happens when the non boot CPUs are brought
>> down, which is the very last step before the actual suspend happens.
>>
>> We could probably do that earlier, but that's something Rafael needs to
>> answer ultimately.
>
> Well, that's both flattering and concerning. ;-)
>
> Anyway, yes, we can do that earlier AFAICS.  Action handlers are not going to
> run after we've called suspend_device_irqs() which happens before the final
> stage of PCI devices suspend (suspend_noirq) and it doesn't matter which CPU
> gets the interrupt from that point on (it is either wakeup or unwanted then).

There is a catch that we don't and likely should not do that for
suspend-to-idle, but since we have pm_suspend_target_state now, that
case can be distinguished from the "full suspend" one readily.

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Wed, Dec 13, 2017 at 11:39 PM, Rafael J. Wysocki  wrote:
> On Wednesday, December 13, 2017 7:19:17 PM CET Thomas Gleixner wrote:
>> On Wed, 13 Dec 2017, Linus Torvalds wrote:
>>
>> > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
>> > wrote:
>> > >
>> > > Definitely. That was fragile forever but puzzles me is that I can't 
>> > > figure
>> > > out what now causes that spurious interrupt to surface out of the blue.
>> >
>> > Perhaps just timing?
>>
>> That's what I'm trying to figure out right now, because that is the only
>> sensible explanation left. The whole machinery of suspend is exactly the
>> same with and without the vector changes. I instrumented all functions
>> involved and the picture is the same. I even do not see any fundamental
>> timing differences where one would say: That's it.
>>
>> What puzzles me even more is that in the range of commits I'm fiddling with
>> there is no other change than the vector management stuff and the point
>> where it breaks makes no sense at all. The point Maarten bisected it to
>> works nicely here, so that might just point to a very subtle timing issue.
>>
>> > How hard would it be to change the ordering to just redirect irqs first?
>>
>> The whole interrupt redirection happens when the non boot CPUs are brought
>> down, which is the very last step before the actual suspend happens.
>>
>> We could probably do that earlier, but that's something Rafael needs to
>> answer ultimately.
>
> Well, that's both flattering and concerning. ;-)
>
> Anyway, yes, we can do that earlier AFAICS.  Action handlers are not going to
> run after we've called suspend_device_irqs() which happens before the final
> stage of PCI devices suspend (suspend_noirq) and it doesn't matter which CPU
> gets the interrupt from that point on (it is either wakeup or unwanted then).

There is a catch that we don't and likely should not do that for
suspend-to-idle, but since we have pm_suspend_target_state now, that
case can be distinguished from the "full suspend" one readily.

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Wednesday, December 13, 2017 10:06:40 PM CET Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > > 
> > > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > > wrote:
> > > > >
> > > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > > figure
> > > > > out what now causes that spurious interrupt to surface out of the 
> > > > > blue.
> > > > 
> > > > Perhaps just timing?
> > > 
> > > That's what I'm trying to figure out right now, because that is the only
> > > sensible explanation left. The whole machinery of suspend is exactly the
> > > same with and without the vector changes. I instrumented all functions
> > > involved and the picture is the same. I even do not see any fundamental
> > > timing differences where one would say: That's it.
> > > 
> > > What puzzles me even more is that in the range of commits I'm fiddling 
> > > with
> > > there is no other change than the vector management stuff and the point
> > > where it breaks makes no sense at all. The point Maarten bisected it to
> > > works nicely here, so that might just point to a very subtle timing issue.
> > 
> > After doing more debugging on this it turns out that this looks like a
> > legacy interrupt coming in. The vector number is always 55, which is legacy
> > IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> > masked and vector 55 is completely unused.
> > 
> > More questions than answers. Still investigating.
> 
> And it does not explain Maartens report which gets a spurious vector 33 on
> CPU4 after the non boot cpus have been brought online again. And that's the
> vector which was assigned before the affinity was moved by unplugging CPU4.
> 
> Hrmpf. Even more mystery to solve.

Any chance to look at /proc/interrupts from a machine where that can be
reproduced?

I'm also curious if that can be reproduced by doing CPU offline/online
without suspending?

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Wednesday, December 13, 2017 10:06:40 PM CET Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > > 
> > > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > > wrote:
> > > > >
> > > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > > figure
> > > > > out what now causes that spurious interrupt to surface out of the 
> > > > > blue.
> > > > 
> > > > Perhaps just timing?
> > > 
> > > That's what I'm trying to figure out right now, because that is the only
> > > sensible explanation left. The whole machinery of suspend is exactly the
> > > same with and without the vector changes. I instrumented all functions
> > > involved and the picture is the same. I even do not see any fundamental
> > > timing differences where one would say: That's it.
> > > 
> > > What puzzles me even more is that in the range of commits I'm fiddling 
> > > with
> > > there is no other change than the vector management stuff and the point
> > > where it breaks makes no sense at all. The point Maarten bisected it to
> > > works nicely here, so that might just point to a very subtle timing issue.
> > 
> > After doing more debugging on this it turns out that this looks like a
> > legacy interrupt coming in. The vector number is always 55, which is legacy
> > IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> > masked and vector 55 is completely unused.
> > 
> > More questions than answers. Still investigating.
> 
> And it does not explain Maartens report which gets a spurious vector 33 on
> CPU4 after the non boot cpus have been brought online again. And that's the
> vector which was assigned before the affinity was moved by unplugging CPU4.
> 
> Hrmpf. Even more mystery to solve.

Any chance to look at /proc/interrupts from a machine where that can be
reproduced?

I'm also curious if that can be reproduced by doing CPU offline/online
without suspending?

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Wednesday, December 13, 2017 7:19:17 PM CET Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Linus Torvalds wrote:
> 
> > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
> > >
> > > Definitely. That was fragile forever but puzzles me is that I can't figure
> > > out what now causes that spurious interrupt to surface out of the blue.
> > 
> > Perhaps just timing?
> 
> That's what I'm trying to figure out right now, because that is the only
> sensible explanation left. The whole machinery of suspend is exactly the
> same with and without the vector changes. I instrumented all functions
> involved and the picture is the same. I even do not see any fundamental
> timing differences where one would say: That's it.
> 
> What puzzles me even more is that in the range of commits I'm fiddling with
> there is no other change than the vector management stuff and the point
> where it breaks makes no sense at all. The point Maarten bisected it to
> works nicely here, so that might just point to a very subtle timing issue.
> 
> > How hard would it be to change the ordering to just redirect irqs first?
> 
> The whole interrupt redirection happens when the non boot CPUs are brought
> down, which is the very last step before the actual suspend happens.
> 
> We could probably do that earlier, but that's something Rafael needs to
> answer ultimately.

Well, that's both flattering and concerning. ;-)

Anyway, yes, we can do that earlier AFAICS.  Action handlers are not going to
run after we've called suspend_device_irqs() which happens before the final
stage of PCI devices suspend (suspend_noirq) and it doesn't matter which CPU
gets the interrupt from that point on (it is either wakeup or unwanted then).

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Rafael J. Wysocki]

On Wednesday, December 13, 2017 7:19:17 PM CET Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Linus Torvalds wrote:
> 
> > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
> > >
> > > Definitely. That was fragile forever but puzzles me is that I can't figure
> > > out what now causes that spurious interrupt to surface out of the blue.
> > 
> > Perhaps just timing?
> 
> That's what I'm trying to figure out right now, because that is the only
> sensible explanation left. The whole machinery of suspend is exactly the
> same with and without the vector changes. I instrumented all functions
> involved and the picture is the same. I even do not see any fundamental
> timing differences where one would say: That's it.
> 
> What puzzles me even more is that in the range of commits I'm fiddling with
> there is no other change than the vector management stuff and the point
> where it breaks makes no sense at all. The point Maarten bisected it to
> works nicely here, so that might just point to a very subtle timing issue.
> 
> > How hard would it be to change the ordering to just redirect irqs first?
> 
> The whole interrupt redirection happens when the non boot CPUs are brought
> down, which is the very last step before the actual suspend happens.
> 
> We could probably do that earlier, but that's something Rafael needs to
> answer ultimately.

Well, that's both flattering and concerning. ;-)

Anyway, yes, we can do that earlier AFAICS.  Action handlers are not going to
run after we've called suspend_device_irqs() which happens before the final
stage of PCI devices suspend (suspend_noirq) and it doesn't matter which CPU
gets the interrupt from that point on (it is either wakeup or unwanted then).

Thanks,
Rafael

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > 
> > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > wrote:
> > > >
> > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > figure
> > > > out what now causes that spurious interrupt to surface out of the blue.
> > > 
> > > Perhaps just timing?
> > 
> > That's what I'm trying to figure out right now, because that is the only
> > sensible explanation left. The whole machinery of suspend is exactly the
> > same with and without the vector changes. I instrumented all functions
> > involved and the picture is the same. I even do not see any fundamental
> > timing differences where one would say: That's it.
> > 
> > What puzzles me even more is that in the range of commits I'm fiddling with
> > there is no other change than the vector management stuff and the point
> > where it breaks makes no sense at all. The point Maarten bisected it to
> > works nicely here, so that might just point to a very subtle timing issue.
> 
> After doing more debugging on this it turns out that this looks like a
> legacy interrupt coming in. The vector number is always 55, which is legacy
> IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> masked and vector 55 is completely unused.
> 
> More questions than answers. Still investigating.

And it does not explain Maartens report which gets a spurious vector 33 on
CPU4 after the non boot cpus have been brought online again. And that's the
vector which was assigned before the affinity was moved by unplugging CPU4.

Hrmpf. Even more mystery to solve.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> > On Wed, 13 Dec 2017, Linus Torvalds wrote:
> > 
> > > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  
> > > wrote:
> > > >
> > > > Definitely. That was fragile forever but puzzles me is that I can't 
> > > > figure
> > > > out what now causes that spurious interrupt to surface out of the blue.
> > > 
> > > Perhaps just timing?
> > 
> > That's what I'm trying to figure out right now, because that is the only
> > sensible explanation left. The whole machinery of suspend is exactly the
> > same with and without the vector changes. I instrumented all functions
> > involved and the picture is the same. I even do not see any fundamental
> > timing differences where one would say: That's it.
> > 
> > What puzzles me even more is that in the range of commits I'm fiddling with
> > there is no other change than the vector management stuff and the point
> > where it breaks makes no sense at all. The point Maarten bisected it to
> > works nicely here, so that might just point to a very subtle timing issue.
> 
> After doing more debugging on this it turns out that this looks like a
> legacy interrupt coming in. The vector number is always 55, which is legacy
> IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
> masked and vector 55 is completely unused.
> 
> More questions than answers. Still investigating.

And it does not explain Maartens report which gets a spurious vector 33 on
CPU4 after the non boot cpus have been brought online again. And that's the
vector which was assigned before the affinity was moved by unplugging CPU4.

Hrmpf. Even more mystery to solve.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Linus Torvalds wrote:
> 
> > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
> > >
> > > Definitely. That was fragile forever but puzzles me is that I can't figure
> > > out what now causes that spurious interrupt to surface out of the blue.
> > 
> > Perhaps just timing?
> 
> That's what I'm trying to figure out right now, because that is the only
> sensible explanation left. The whole machinery of suspend is exactly the
> same with and without the vector changes. I instrumented all functions
> involved and the picture is the same. I even do not see any fundamental
> timing differences where one would say: That's it.
> 
> What puzzles me even more is that in the range of commits I'm fiddling with
> there is no other change than the vector management stuff and the point
> where it breaks makes no sense at all. The point Maarten bisected it to
> works nicely here, so that might just point to a very subtle timing issue.

After doing more debugging on this it turns out that this looks like a
legacy interrupt coming in. The vector number is always 55, which is legacy
IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
masked and vector 55 is completely unused.

More questions than answers. Still investigating.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Thomas Gleixner wrote:
> On Wed, 13 Dec 2017, Linus Torvalds wrote:
> 
> > On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
> > >
> > > Definitely. That was fragile forever but puzzles me is that I can't figure
> > > out what now causes that spurious interrupt to surface out of the blue.
> > 
> > Perhaps just timing?
> 
> That's what I'm trying to figure out right now, because that is the only
> sensible explanation left. The whole machinery of suspend is exactly the
> same with and without the vector changes. I instrumented all functions
> involved and the picture is the same. I even do not see any fundamental
> timing differences where one would say: That's it.
> 
> What puzzles me even more is that in the range of commits I'm fiddling with
> there is no other change than the vector management stuff and the point
> where it breaks makes no sense at all. The point Maarten bisected it to
> works nicely here, so that might just point to a very subtle timing issue.

After doing more debugging on this it turns out that this looks like a
legacy interrupt coming in. The vector number is always 55, which is legacy
IRQ 7 as seen from the PIC. The corresponding IOAPIC interrupt pin is
masked and vector 55 is completely unused.

More questions than answers. Still investigating.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Wed, Dec 13, 2017 at 3:16 AM, Jarkko Nikula
 wrote:
> On 12/13/2017 12:10 AM, Andy Lutomirski wrote:
>>
>> On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
>>  wrote:
>>>
>>> On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski 
>>> wrote:
>>
>> Like this?
>>
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856
>>
>> I've barely tested it.  It suspended and resumed once in a 64-bit VM.
>> It compiles on 32-bit.
>>
> I tested with "rtcwake -s 5 -m mem" 2-3 times on a real machine. Both 64-bit
> and 32-bit work.
>
> Tested-by: Jarkko Nikula 

Thanks!

I've split the patch into three and put your Tested-By on all of them,
since the end state is exactly the same as what you tested.  I'll
email them out once I test them myself :)

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Wed, Dec 13, 2017 at 3:16 AM, Jarkko Nikula
 wrote:
> On 12/13/2017 12:10 AM, Andy Lutomirski wrote:
>>
>> On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
>>  wrote:
>>>
>>> On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski 
>>> wrote:
>>
>> Like this?
>>
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856
>>
>> I've barely tested it.  It suspended and resumed once in a 64-bit VM.
>> It compiles on 32-bit.
>>
> I tested with "rtcwake -s 5 -m mem" 2-3 times on a real machine. Both 64-bit
> and 32-bit work.
>
> Tested-by: Jarkko Nikula 

Thanks!

I've split the patch into three and put your Tested-By on all of them,
since the end state is exactly the same as what you tested.  I'll
email them out once I test them myself :)

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Linus Torvalds wrote:

> On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
> >
> > Definitely. That was fragile forever but puzzles me is that I can't figure
> > out what now causes that spurious interrupt to surface out of the blue.
> 
> Perhaps just timing?

That's what I'm trying to figure out right now, because that is the only
sensible explanation left. The whole machinery of suspend is exactly the
same with and without the vector changes. I instrumented all functions
involved and the picture is the same. I even do not see any fundamental
timing differences where one would say: That's it.

What puzzles me even more is that in the range of commits I'm fiddling with
there is no other change than the vector management stuff and the point
where it breaks makes no sense at all. The point Maarten bisected it to
works nicely here, so that might just point to a very subtle timing issue.

> How hard would it be to change the ordering to just redirect irqs first?

The whole interrupt redirection happens when the non boot CPUs are brought
down, which is the very last step before the actual suspend happens.

We could probably do that earlier, but that's something Rafael needs to
answer ultimately.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Linus Torvalds wrote:

> On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
> >
> > Definitely. That was fragile forever but puzzles me is that I can't figure
> > out what now causes that spurious interrupt to surface out of the blue.
> 
> Perhaps just timing?

That's what I'm trying to figure out right now, because that is the only
sensible explanation left. The whole machinery of suspend is exactly the
same with and without the vector changes. I instrumented all functions
involved and the picture is the same. I even do not see any fundamental
timing differences where one would say: That's it.

What puzzles me even more is that in the range of commits I'm fiddling with
there is no other change than the vector management stuff and the point
where it breaks makes no sense at all. The point Maarten bisected it to
works nicely here, so that might just point to a very subtle timing issue.

> How hard would it be to change the ordering to just redirect irqs first?

The whole interrupt redirection happens when the non boot CPUs are brought
down, which is the very last step before the actual suspend happens.

We could probably do that earlier, but that's something Rafael needs to
answer ultimately.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
>
> Definitely. That was fragile forever but puzzles me is that I can't figure
> out what now causes that spurious interrupt to surface out of the blue.

Perhaps just timing?

How hard would it be to change the ordering to just redirect irqs first?

 Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Wed, Dec 13, 2017 at 8:41 AM, Thomas Gleixner  wrote:
>
> Definitely. That was fragile forever but puzzles me is that I can't figure
> out what now causes that spurious interrupt to surface out of the blue.

Perhaps just timing?

How hard would it be to change the ordering to just redirect irqs first?

 Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Bjorn Helgaas wrote:
> [+cc linux-pci, linux-pm]
> 
> On Wed, Dec 13, 2017 at 04:57:56PM +0100, Thomas Gleixner wrote:
> > So I was finally able to figure out what the hell is going on:
> > 
> > Suspend:
> > 
> >  - The device suspend code puts the graphics card into a power
> >state != PCI_D0.
> > 
> >  - Offline non boot CPUs
> > 
> >  - Break interrupt affinity. Allocate new vector on CPU 0, compose and
> >write MSI message which ends up in:
> > 
> >__pci_write_msi_msg(entry, msg)
> >{
> > if (dev->current_state != PCI_D0 || pci_dev_is_disconnected(dev)) {
> >/* Don't touch the hardware now */
> > } else {
> >
> > }
> > entry->msg = *msg;
> >}
> >  
> >   So because the device is not in PCI_D0 the message is not written. It's
> >   written in the device resume path.
> 
> I'm not a PM guru, but this ordering seems fragile.  If we offline
> CPUs before re-targeting interrupts directed at those CPUs, aren't we
> always going to be at risk of sending interrupts to an offline CPU?
> 
> Even if the device is now asleep and therefore should not generate an
> interrupt, it seems like there's a window when the device returns to
> PCI_D0 where it could generate an interrupt before we have a chance to
> update the MSI message.

Definitely. That was fragile forever but puzzles me is that I can't figure
out what now causes that spurious interrupt to surface out of the blue.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Wed, 13 Dec 2017, Bjorn Helgaas wrote:
> [+cc linux-pci, linux-pm]
> 
> On Wed, Dec 13, 2017 at 04:57:56PM +0100, Thomas Gleixner wrote:
> > So I was finally able to figure out what the hell is going on:
> > 
> > Suspend:
> > 
> >  - The device suspend code puts the graphics card into a power
> >state != PCI_D0.
> > 
> >  - Offline non boot CPUs
> > 
> >  - Break interrupt affinity. Allocate new vector on CPU 0, compose and
> >write MSI message which ends up in:
> > 
> >__pci_write_msi_msg(entry, msg)
> >{
> > if (dev->current_state != PCI_D0 || pci_dev_is_disconnected(dev)) {
> >/* Don't touch the hardware now */
> > } else {
> >
> > }
> > entry->msg = *msg;
> >}
> >  
> >   So because the device is not in PCI_D0 the message is not written. It's
> >   written in the device resume path.
> 
> I'm not a PM guru, but this ordering seems fragile.  If we offline
> CPUs before re-targeting interrupts directed at those CPUs, aren't we
> always going to be at risk of sending interrupts to an offline CPU?
> 
> Even if the device is now asleep and therefore should not generate an
> interrupt, it seems like there's a window when the device returns to
> PCI_D0 where it could generate an interrupt before we have a chance to
> update the MSI message.

Definitely. That was fragile forever but puzzles me is that I can't figure
out what now causes that spurious interrupt to surface out of the blue.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Bjorn Helgaas]

[+cc linux-pci, linux-pm]

On Wed, Dec 13, 2017 at 04:57:56PM +0100, Thomas Gleixner wrote:
> So I was finally able to figure out what the hell is going on:
> 
> Suspend:
> 
>  - The device suspend code puts the graphics card into a power
>state != PCI_D0.
> 
>  - Offline non boot CPUs
> 
>  - Break interrupt affinity. Allocate new vector on CPU 0, compose and
>write MSI message which ends up in:
> 
>__pci_write_msi_msg(entry, msg)
>{
>   if (dev->current_state != PCI_D0 || pci_dev_is_disconnected(dev)) {
>  /* Don't touch the hardware now */
>   } else {
>  
>   }
>   entry->msg = *msg;
>}
>  
>   So because the device is not in PCI_D0 the message is not written. It's
>   written in the device resume path.

I'm not a PM guru, but this ordering seems fragile.  If we offline
CPUs before re-targeting interrupts directed at those CPUs, aren't we
always going to be at risk of sending interrupts to an offline CPU?

Even if the device is now asleep and therefore should not generate an
interrupt, it seems like there's a window when the device returns to
PCI_D0 where it could generate an interrupt before we have a chance to
update the MSI message.

> Resume:
> [  139.670446] ACPI: Low-level resume complete
> [  139.670541] PM: Restoring platform NVS memory
> [  139.672462] do_IRQ: 0.55 No irq handler for vector
> [  139.672475] Enabling non-boot CPUs ...
> 
> So the spurious interrupt happens early and way before the device resume
> code writes the new MSI message.
> 
> I checked the behaviour on 4.14. The MSI write is delayed there in the same
> way, but there is no spurious interrupt. There is no interrupt coming in at
> all _BEFORE_ the device is put out of PCI_D0.
> 
> And this has certainly nothing to do with the vector management changes,
> but I can't figure yet what makes that spurious interrupt to be sent.
> 
> Any ideas welcome.
> 
> Thanks,
> 
>   tglx
> 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Bjorn Helgaas]

[+cc linux-pci, linux-pm]

On Wed, Dec 13, 2017 at 04:57:56PM +0100, Thomas Gleixner wrote:
> So I was finally able to figure out what the hell is going on:
> 
> Suspend:
> 
>  - The device suspend code puts the graphics card into a power
>state != PCI_D0.
> 
>  - Offline non boot CPUs
> 
>  - Break interrupt affinity. Allocate new vector on CPU 0, compose and
>write MSI message which ends up in:
> 
>__pci_write_msi_msg(entry, msg)
>{
>   if (dev->current_state != PCI_D0 || pci_dev_is_disconnected(dev)) {
>  /* Don't touch the hardware now */
>   } else {
>  
>   }
>   entry->msg = *msg;
>}
>  
>   So because the device is not in PCI_D0 the message is not written. It's
>   written in the device resume path.

I'm not a PM guru, but this ordering seems fragile.  If we offline
CPUs before re-targeting interrupts directed at those CPUs, aren't we
always going to be at risk of sending interrupts to an offline CPU?

Even if the device is now asleep and therefore should not generate an
interrupt, it seems like there's a window when the device returns to
PCI_D0 where it could generate an interrupt before we have a chance to
update the MSI message.

> Resume:
> [  139.670446] ACPI: Low-level resume complete
> [  139.670541] PM: Restoring platform NVS memory
> [  139.672462] do_IRQ: 0.55 No irq handler for vector
> [  139.672475] Enabling non-boot CPUs ...
> 
> So the spurious interrupt happens early and way before the device resume
> code writes the new MSI message.
> 
> I checked the behaviour on 4.14. The MSI write is delayed there in the same
> way, but there is no spurious interrupt. There is no interrupt coming in at
> all _BEFORE_ the device is put out of PCI_D0.
> 
> And this has certainly nothing to do with the vector management changes,
> but I can't figure yet what makes that spurious interrupt to be sent.
> 
> Any ideas welcome.
> 
> Thanks,
> 
>   tglx
> 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

So I was finally able to figure out what the hell is going on:

Suspend:

 - The device suspend code puts the graphics card into a power
   state != PCI_D0.

 - Offline non boot CPUs

 - Break interrupt affinity. Allocate new vector on CPU 0, compose and
   write MSI message which ends up in:

   __pci_write_msi_msg(entry, msg)
   {
if (dev->current_state != PCI_D0 || pci_dev_is_disconnected(dev)) {
   /* Don't touch the hardware now */
} else {
   
}
entry->msg = *msg;
   }
 
  So because the device is not in PCI_D0 the message is not written. It's
  written in the device resume path.

Resume:
[  139.670446] ACPI: Low-level resume complete
[  139.670541] PM: Restoring platform NVS memory
[  139.672462] do_IRQ: 0.55 No irq handler for vector
[  139.672475] Enabling non-boot CPUs ...

So the spurious interrupt happens early and way before the device resume
code writes the new MSI message.

I checked the behaviour on 4.14. The MSI write is delayed there in the same
way, but there is no spurious interrupt. There is no interrupt coming in at
all _BEFORE_ the device is put out of PCI_D0.

And this has certainly nothing to do with the vector management changes,
but I can't figure yet what makes that spurious interrupt to be sent.

Any ideas welcome.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

So I was finally able to figure out what the hell is going on:

Suspend:

 - The device suspend code puts the graphics card into a power
   state != PCI_D0.

 - Offline non boot CPUs

 - Break interrupt affinity. Allocate new vector on CPU 0, compose and
   write MSI message which ends up in:

   __pci_write_msi_msg(entry, msg)
   {
if (dev->current_state != PCI_D0 || pci_dev_is_disconnected(dev)) {
   /* Don't touch the hardware now */
} else {
   
}
entry->msg = *msg;
   }
 
  So because the device is not in PCI_D0 the message is not written. It's
  written in the device resume path.

Resume:
[  139.670446] ACPI: Low-level resume complete
[  139.670541] PM: Restoring platform NVS memory
[  139.672462] do_IRQ: 0.55 No irq handler for vector
[  139.672475] Enabling non-boot CPUs ...

So the spurious interrupt happens early and way before the device resume
code writes the new MSI message.

I checked the behaviour on 4.14. The MSI write is delayed there in the same
way, but there is no spurious interrupt. There is no interrupt coming in at
all _BEFORE_ the device is put out of PCI_D0.

And this has certainly nothing to do with the vector management changes,
but I can't figure yet what makes that spurious interrupt to be sent.

Any ideas welcome.

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Ingo Molnar]

* Jarkko Nikula  wrote:

> On 12/13/2017 12:10 AM, Andy Lutomirski wrote:
> > On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
> >  wrote:
> > > On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:
> > Like this?
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856
> > 
> > I've barely tested it.  It suspended and resumed once in a 64-bit VM.
> > It compiles on 32-bit.
> > 
> I tested with "rtcwake -s 5 -m mem" 2-3 times on a real machine. Both 64-bit
> and 32-bit work.
> 
> Tested-by: Jarkko Nikula 

Great, thanks for the testing!

Andy, mind sending the final fix with a changelog and the Reported-by/Tested-by 
tags, etc?

Thanks,

Ingo

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Ingo Molnar]

* Jarkko Nikula  wrote:

> On 12/13/2017 12:10 AM, Andy Lutomirski wrote:
> > On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
> >  wrote:
> > > On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:
> > Like this?
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856
> > 
> > I've barely tested it.  It suspended and resumed once in a 64-bit VM.
> > It compiles on 32-bit.
> > 
> I tested with "rtcwake -s 5 -m mem" 2-3 times on a real machine. Both 64-bit
> and 32-bit work.
> 
> Tested-by: Jarkko Nikula 

Great, thanks for the testing!

Andy, mind sending the final fix with a changelog and the Reported-by/Tested-by 
tags, etc?

Thanks,

Ingo

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Jarkko Nikula]

On 12/13/2017 12:10 AM, Andy Lutomirski wrote:

On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
 wrote:

On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:

Like this?

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856

I've barely tested it.  It suspended and resumed once in a 64-bit VM.
It compiles on 32-bit.

I tested with "rtcwake -s 5 -m mem" 2-3 times on a real machine. Both 
64-bit and 32-bit work.


Tested-by: Jarkko Nikula 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Jarkko Nikula]

On 12/13/2017 12:10 AM, Andy Lutomirski wrote:

On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
 wrote:

On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:

Like this?

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856

I've barely tested it.  It suspended and resumed once in a 64-bit VM.
It compiles on 32-bit.

I tested with "rtcwake -s 5 -m mem" 2-3 times on a real machine. Both 
64-bit and 32-bit work.


Tested-by: Jarkko Nikula 

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Tue, Dec 12, 2017 at 2:33 PM, Linus Torvalds
 wrote:
> On Tue, Dec 12, 2017 at 2:10 PM, Andy Lutomirski  wrote:
>>
>> (That link might not work for a little bit.  I'm not sure what's up.)
>
> I think your link is just bogus.
>
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes
>
> works.

If I click "commit" on that page, I get my link, and it's still busted.

>
> Anyway, the code looks much better to me.
>
> Whether it _works_ is another matter, of course.
>
> Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Tue, Dec 12, 2017 at 2:33 PM, Linus Torvalds
 wrote:
> On Tue, Dec 12, 2017 at 2:10 PM, Andy Lutomirski  wrote:
>>
>> (That link might not work for a little bit.  I'm not sure what's up.)
>
> I think your link is just bogus.
>
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes
>
> works.

If I click "commit" on that page, I get my link, and it's still busted.

>
> Anyway, the code looks much better to me.
>
> Whether it _works_ is another matter, of course.
>
> Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Tue, Dec 12, 2017 at 2:10 PM, Andy Lutomirski  wrote:
>
> (That link might not work for a little bit.  I'm not sure what's up.)

I think your link is just bogus.

  
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes

works.

Anyway, the code looks much better to me.

Whether it _works_ is another matter, of course.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Tue, Dec 12, 2017 at 2:10 PM, Andy Lutomirski  wrote:
>
> (That link might not work for a little bit.  I'm not sure what's up.)

I think your link is just bogus.

  
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes

works.

Anyway, the code looks much better to me.

Whether it _works_ is another matter, of course.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
 wrote:
> On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:
>>>
>>> - do NOT use "load_gs_index()", which does that swapgs dance (twice!)
>>> and plays with interrupt state. Just load the segment register, and
>>> then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
>>> need for the swapgs dance.
>>
>> Using what helper?  On x86_64, it can fault, and IIRC we explicitly
>> don't allow loadsegment(gs, ...).
>
> Just do the loadsegment() thing. The fact that we don't have a gs
> version of it is legacy - to catch bad users. It shouldn't stop us
> from having good users.
>
> That said - can it really fault? Because if it can, then why can't %fs
> fault? And on x86-64, we just do
>
> asm volatile ("movw %0, %%fs" :: "r" (ctxt->fs));
>
> and don't actually use 'loadsegment()' for _any_ of the segments.  We
> only do the fault protection on 32-bit.
>
> In fact, we really should try to avoid taking faults here anyway,
> shouldn't we? We haven't loaded enough of the context yet.
>
> Hmm.
>
> Maybe we should load only the fixed kernel segments at this point, and
> then do all the loadsegment() of gs/fs in the later phase when we're
> all set up.
>
> THERE we can do the swapgs dance with interrupt tracing etc, because
> *there* we actually are fully set up. I guess that means reloading the
> FS/GS base MSR's,

Like this?

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856

I've barely tested it.  It suspended and resumed once in a 64-bit VM.
It compiles on 32-bit.

(That link might not work for a little bit.  I'm not sure what's up.)

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Tue, Dec 12, 2017 at 10:36 AM, Linus Torvalds
 wrote:
> On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:
>>>
>>> - do NOT use "load_gs_index()", which does that swapgs dance (twice!)
>>> and plays with interrupt state. Just load the segment register, and
>>> then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
>>> need for the swapgs dance.
>>
>> Using what helper?  On x86_64, it can fault, and IIRC we explicitly
>> don't allow loadsegment(gs, ...).
>
> Just do the loadsegment() thing. The fact that we don't have a gs
> version of it is legacy - to catch bad users. It shouldn't stop us
> from having good users.
>
> That said - can it really fault? Because if it can, then why can't %fs
> fault? And on x86-64, we just do
>
> asm volatile ("movw %0, %%fs" :: "r" (ctxt->fs));
>
> and don't actually use 'loadsegment()' for _any_ of the segments.  We
> only do the fault protection on 32-bit.
>
> In fact, we really should try to avoid taking faults here anyway,
> shouldn't we? We haven't loaded enough of the context yet.
>
> Hmm.
>
> Maybe we should load only the fixed kernel segments at this point, and
> then do all the loadsegment() of gs/fs in the later phase when we're
> all set up.
>
> THERE we can do the swapgs dance with interrupt tracing etc, because
> *there* we actually are fully set up. I guess that means reloading the
> FS/GS base MSR's,

Like this?

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes=cb855aa9679a15adbe43732f5854270de2b35856

I've barely tested it.  It suspended and resumed once in a 64-bit VM.
It compiles on 32-bit.

(That link might not work for a little bit.  I'm not sure what's up.)

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:
>>
>> - do NOT use "load_gs_index()", which does that swapgs dance (twice!)
>> and plays with interrupt state. Just load the segment register, and
>> then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
>> need for the swapgs dance.
>
> Using what helper?  On x86_64, it can fault, and IIRC we explicitly
> don't allow loadsegment(gs, ...).

Just do the loadsegment() thing. The fact that we don't have a gs
version of it is legacy - to catch bad users. It shouldn't stop us
from having good users.

That said - can it really fault? Because if it can, then why can't %fs
fault? And on x86-64, we just do

asm volatile ("movw %0, %%fs" :: "r" (ctxt->fs));

and don't actually use 'loadsegment()' for _any_ of the segments.  We
only do the fault protection on 32-bit.

In fact, we really should try to avoid taking faults here anyway,
shouldn't we? We haven't loaded enough of the context yet.

Hmm.

Maybe we should load only the fixed kernel segments at this point, and
then do all the loadsegment() of gs/fs in the later phase when we're
all set up.

THERE we can do the swapgs dance with interrupt tracing etc, because
*there* we actually are fully set up. I guess that means reloading the
FS/GS base MSR's,

  Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski  wrote:
>>
>> - do NOT use "load_gs_index()", which does that swapgs dance (twice!)
>> and plays with interrupt state. Just load the segment register, and
>> then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
>> need for the swapgs dance.
>
> Using what helper?  On x86_64, it can fault, and IIRC we explicitly
> don't allow loadsegment(gs, ...).

Just do the loadsegment() thing. The fact that we don't have a gs
version of it is legacy - to catch bad users. It shouldn't stop us
from having good users.

That said - can it really fault? Because if it can, then why can't %fs
fault? And on x86-64, we just do

asm volatile ("movw %0, %%fs" :: "r" (ctxt->fs));

and don't actually use 'loadsegment()' for _any_ of the segments.  We
only do the fault protection on 32-bit.

In fact, we really should try to avoid taking faults here anyway,
shouldn't we? We haven't loaded enough of the context yet.

Hmm.

Maybe we should load only the fixed kernel segments at this point, and
then do all the loadsegment() of gs/fs in the later phase when we're
all set up.

THERE we can do the swapgs dance with interrupt tracing etc, because
*there* we actually are fully set up. I guess that means reloading the
FS/GS base MSR's,

  Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Tue, Dec 12, 2017 at 9:27 AM, Linus Torvalds
 wrote:
> On Sun, Dec 10, 2017 at 1:28 PM, Linus Torvalds
>  wrote:
>>
>> That said, this *all* smells wrong. Why is there a special
>> fix_processor_context() function at all with different 32-bit and
>> 64-bit behavior? This code is all written to be maximally confusing.
>
> Hmm. Looking a bit more at this, I think it should be solved by:
>
>  - load the original read-write GDT early, along with the IDT.
>
>   We have already saved it off in __save_processor_state(), and it may
> have already gotten loaded very early in at least some of the paths,
> but it definitely hasn't gotten reloaded in all the paths (think
> "suspend/resume testing" etc).
>
>  - add the LDT descriptor to the save area too, exactly like we
> already have IDT/GDT.
>
>Then, we can do "load_ldt()" early (along with IDT and GDT).
>
>  - now we can just load all the segments early, and get the percpu and
> stack canary stuff without any special cases
>
> - do NOT use "load_gs_index()", which does that swapgs dance (twice!)
> and plays with interrupt state. Just load the segment register, and
> then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
> need for the swapgs dance.

Using what helper?  On x86_64, it can fault, and IIRC we explicitly
don't allow loadsegment(gs, ...).

>
>  - now that we have a fully working system, then the
> "fix_processor_context()" code can do the "fancy" stuff like setting
> up the RO fixmap GDT and re-initializing the TLB state. Those want
> percpu stuff.
>
> In other words, why not try to organize this so that we do a simple
> and fairly mindless restore of the low-level state first? Avoid all
> the "system is halfway up" crud.
>
> Would that work for people? Andy?

Other than the above, more or less.

But we should really do all the user segments last.  They're not at
all needed for normal kernel execution, so I think they should be the
very last part.

I'll try to get to this today.

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Tue, Dec 12, 2017 at 9:27 AM, Linus Torvalds
 wrote:
> On Sun, Dec 10, 2017 at 1:28 PM, Linus Torvalds
>  wrote:
>>
>> That said, this *all* smells wrong. Why is there a special
>> fix_processor_context() function at all with different 32-bit and
>> 64-bit behavior? This code is all written to be maximally confusing.
>
> Hmm. Looking a bit more at this, I think it should be solved by:
>
>  - load the original read-write GDT early, along with the IDT.
>
>   We have already saved it off in __save_processor_state(), and it may
> have already gotten loaded very early in at least some of the paths,
> but it definitely hasn't gotten reloaded in all the paths (think
> "suspend/resume testing" etc).
>
>  - add the LDT descriptor to the save area too, exactly like we
> already have IDT/GDT.
>
>Then, we can do "load_ldt()" early (along with IDT and GDT).
>
>  - now we can just load all the segments early, and get the percpu and
> stack canary stuff without any special cases
>
> - do NOT use "load_gs_index()", which does that swapgs dance (twice!)
> and plays with interrupt state. Just load the segment register, and
> then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
> need for the swapgs dance.

Using what helper?  On x86_64, it can fault, and IIRC we explicitly
don't allow loadsegment(gs, ...).

>
>  - now that we have a fully working system, then the
> "fix_processor_context()" code can do the "fancy" stuff like setting
> up the RO fixmap GDT and re-initializing the TLB state. Those want
> percpu stuff.
>
> In other words, why not try to organize this so that we do a simple
> and fairly mindless restore of the low-level state first? Avoid all
> the "system is halfway up" crud.
>
> Would that work for people? Andy?

Other than the above, more or less.

But we should really do all the user segments last.  They're not at
all needed for normal kernel execution, so I think they should be the
very last part.

I'll try to get to this today.

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 1:28 PM, Linus Torvalds
 wrote:
>
> That said, this *all* smells wrong. Why is there a special
> fix_processor_context() function at all with different 32-bit and
> 64-bit behavior? This code is all written to be maximally confusing.

Hmm. Looking a bit more at this, I think it should be solved by:

 - load the original read-write GDT early, along with the IDT.

  We have already saved it off in __save_processor_state(), and it may
have already gotten loaded very early in at least some of the paths,
but it definitely hasn't gotten reloaded in all the paths (think
"suspend/resume testing" etc).

 - add the LDT descriptor to the save area too, exactly like we
already have IDT/GDT.

   Then, we can do "load_ldt()" early (along with IDT and GDT).

 - now we can just load all the segments early, and get the percpu and
stack canary stuff without any special cases

- do NOT use "load_gs_index()", which does that swapgs dance (twice!)
and plays with interrupt state. Just load the segment register, and
then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
need for the swapgs dance.

 - now that we have a fully working system, then the
"fix_processor_context()" code can do the "fancy" stuff like setting
up the RO fixmap GDT and re-initializing the TLB state. Those want
percpu stuff.

In other words, why not try to organize this so that we do a simple
and fairly mindless restore of the low-level state first? Avoid all
the "system is halfway up" crud.

Would that work for people? Andy?

   Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 1:28 PM, Linus Torvalds
 wrote:
>
> That said, this *all* smells wrong. Why is there a special
> fix_processor_context() function at all with different 32-bit and
> 64-bit behavior? This code is all written to be maximally confusing.

Hmm. Looking a bit more at this, I think it should be solved by:

 - load the original read-write GDT early, along with the IDT.

  We have already saved it off in __save_processor_state(), and it may
have already gotten loaded very early in at least some of the paths,
but it definitely hasn't gotten reloaded in all the paths (think
"suspend/resume testing" etc).

 - add the LDT descriptor to the save area too, exactly like we
already have IDT/GDT.

   Then, we can do "load_ldt()" early (along with IDT and GDT).

 - now we can just load all the segments early, and get the percpu and
stack canary stuff without any special cases

- do NOT use "load_gs_index()", which does that swapgs dance (twice!)
and plays with interrupt state. Just load the segment register, and
then do the wrmsrl() of the {FS,GS,KERNEL_GS}_BASE values. There is no
need for the swapgs dance.

 - now that we have a fully working system, then the
"fix_processor_context()" code can do the "fancy" stuff like setting
up the RO fixmap GDT and re-initializing the TLB state. Those want
percpu stuff.

In other words, why not try to organize this so that we do a simple
and fairly mindless restore of the low-level state first? Avoid all
the "system is halfway up" crud.

Would that work for people? Andy?

   Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

Hi!

> > > ...should take 10 seconds or so.
> > I'm told 0day does *some* suspend/resume testing, but I think it's
> > pretty limited, partly because the kinds of machines it primarily
> > works on don't really support suspend/resume at all.
> 
> currently, we're running suspend test on 1 platform only, with 64 bit
> kernel. suspend test will be enabled on more platforms (laptops) in
> next two weeks.

Thanks!

> >  I'm also not sure
> > just how many of those machines are 32-bit at all..
> 
> for this, I suppose it can be reproduced if we use 32-bit kernel and
> rootfs, right? Then it's easier to enable this in 0Day.

Yes, Intel cpus are pretty good at backwards compatibility, and most
problems are not subtle at all. So yes, 32-bit kernel / rootfs on
recent machine should be good for testing.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

Hi!

> > > ...should take 10 seconds or so.
> > I'm told 0day does *some* suspend/resume testing, but I think it's
> > pretty limited, partly because the kinds of machines it primarily
> > works on don't really support suspend/resume at all.
> 
> currently, we're running suspend test on 1 platform only, with 64 bit
> kernel. suspend test will be enabled on more platforms (laptops) in
> next two weeks.

Thanks!

> >  I'm also not sure
> > just how many of those machines are 32-bit at all..
> 
> for this, I suppose it can be reproduced if we use 32-bit kernel and
> rootfs, right? Then it's easier to enable this in 0Day.

Yes, Intel cpus are pretty good at backwards compatibility, and most
problems are not subtle at all. So yes, 32-bit kernel / rootfs on
recent machine should be good for testing.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Mon, Dec 11, 2017 at 6:09 AM, Zhang Rui  wrote:
> On Sun, 2017-12-10 at 12:30 -0800, Linus Torvalds wrote:
>> On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
>> >
>> >
>> > Confirmed, revert fixes it. You see how it moves
>> > fix_processor_context
>> > around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
>> > machines exist? Aha.
>> Yeah, people do.
>>
>> Andy?
>>
>> >
>> > Which brings me to .. various people do automated testing of
>> > kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit
>> > for
>> > boot and suspend would be very nice. The last item is not hard,
>> > either:
>> >
>> > sudo rtcwake -l -m mem -s 5
>> >
>> > ...should take 10 seconds or so.
>> I'm told 0day does *some* suspend/resume testing, but I think it's
>> pretty limited, partly because the kinds of machines it primarily
>> works on don't really support suspend/resume at all.
>
> currently, we're running suspend test on 1 platform only, with 64 bit
> kernel. suspend test will be enabled on more platforms (laptops) in
> next two weeks.
>
> I will check why it does not find the first regression introduced by
> ca37e57bbe0c ("x86/entry/64: Add missing irqflags tracing to
> native_load_gs_index()").
>
>>  I'm also not sure
>> just how many of those machines are 32-bit at all..
>
> for this, I suppose it can be reproduced if we use 32-bit kernel and
> rootfs, right? Then it's easier to enable this in 0Day.
>

Yes.

The 64-bit problem should also be reproducible with rtcwake even in a vm.

Also, on this topic, could make run_tests in
tools/testing/selftests/x86 be added to the rotation as well?  The
testing dir should match the kernel being tested IMO.

> thanks,
> rui
>>
>> But I'm adding Zhang Rui to the cc, to see if my recollection is
>> right.
>>
>> Because you're right, more suspend/resume automated testing would be
>> good to have. And yes, people test mainly 64-bit these days.
>>
>> Also, I'm not even sure what the 0day rules are for just plain
>> mainline. I don't tend to see a lot of breakage reports, even though
>> I'd expect to. This came in from the x86 trees (and those do their
>> own
>> tests too, but probably not suspend/resume either), but it hit my
>> tree
>> fairly soon after going into the x86 -tip trees.
>>
>> Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Andy Lutomirski]

On Mon, Dec 11, 2017 at 6:09 AM, Zhang Rui  wrote:
> On Sun, 2017-12-10 at 12:30 -0800, Linus Torvalds wrote:
>> On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
>> >
>> >
>> > Confirmed, revert fixes it. You see how it moves
>> > fix_processor_context
>> > around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
>> > machines exist? Aha.
>> Yeah, people do.
>>
>> Andy?
>>
>> >
>> > Which brings me to .. various people do automated testing of
>> > kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit
>> > for
>> > boot and suspend would be very nice. The last item is not hard,
>> > either:
>> >
>> > sudo rtcwake -l -m mem -s 5
>> >
>> > ...should take 10 seconds or so.
>> I'm told 0day does *some* suspend/resume testing, but I think it's
>> pretty limited, partly because the kinds of machines it primarily
>> works on don't really support suspend/resume at all.
>
> currently, we're running suspend test on 1 platform only, with 64 bit
> kernel. suspend test will be enabled on more platforms (laptops) in
> next two weeks.
>
> I will check why it does not find the first regression introduced by
> ca37e57bbe0c ("x86/entry/64: Add missing irqflags tracing to
> native_load_gs_index()").
>
>>  I'm also not sure
>> just how many of those machines are 32-bit at all..
>
> for this, I suppose it can be reproduced if we use 32-bit kernel and
> rootfs, right? Then it's easier to enable this in 0Day.
>

Yes.

The 64-bit problem should also be reproducible with rtcwake even in a vm.

Also, on this topic, could make run_tests in
tools/testing/selftests/x86 be added to the rotation as well?  The
testing dir should match the kernel being tested IMO.

> thanks,
> rui
>>
>> But I'm adding Zhang Rui to the cc, to see if my recollection is
>> right.
>>
>> Because you're right, more suspend/resume automated testing would be
>> good to have. And yes, people test mainly 64-bit these days.
>>
>> Also, I'm not even sure what the 0day rules are for just plain
>> mainline. I don't tend to see a lot of breakage reports, even though
>> I'd expect to. This came in from the x86 trees (and those do their
>> own
>> tests too, but probably not suspend/resume either), but it hit my
>> tree
>> fairly soon after going into the x86 -tip trees.
>>
>> Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Zhang Rui]

On Sun, 2017-12-10 at 12:30 -0800, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
> > 
> > 
> > Confirmed, revert fixes it. You see how it moves
> > fix_processor_context
> > around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
> > machines exist? Aha.
> Yeah, people do.
> 
> Andy?
> 
> > 
> > Which brings me to .. various people do automated testing of
> > kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit
> > for
> > boot and suspend would be very nice. The last item is not hard,
> > either:
> > 
> > sudo rtcwake -l -m mem -s 5
> > 
> > ...should take 10 seconds or so.
> I'm told 0day does *some* suspend/resume testing, but I think it's
> pretty limited, partly because the kinds of machines it primarily
> works on don't really support suspend/resume at all.

currently, we're running suspend test on 1 platform only, with 64 bit
kernel. suspend test will be enabled on more platforms (laptops) in
next two weeks.

I will check why it does not find the first regression introduced by
ca37e57bbe0c ("x86/entry/64: Add missing irqflags tracing to
native_load_gs_index()").

>  I'm also not sure
> just how many of those machines are 32-bit at all..

for this, I suppose it can be reproduced if we use 32-bit kernel and
rootfs, right? Then it's easier to enable this in 0Day.

thanks,
rui
> 
> But I'm adding Zhang Rui to the cc, to see if my recollection is
> right.
> 
> Because you're right, more suspend/resume automated testing would be
> good to have. And yes, people test mainly 64-bit these days.
> 
> Also, I'm not even sure what the 0day rules are for just plain
> mainline. I don't tend to see a lot of breakage reports, even though
> I'd expect to. This came in from the x86 trees (and those do their
> own
> tests too, but probably not suspend/resume either), but it hit my
> tree
> fairly soon after going into the x86 -tip trees.
> 
> Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Zhang Rui]

On Sun, 2017-12-10 at 12:30 -0800, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
> > 
> > 
> > Confirmed, revert fixes it. You see how it moves
> > fix_processor_context
> > around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
> > machines exist? Aha.
> Yeah, people do.
> 
> Andy?
> 
> > 
> > Which brings me to .. various people do automated testing of
> > kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit
> > for
> > boot and suspend would be very nice. The last item is not hard,
> > either:
> > 
> > sudo rtcwake -l -m mem -s 5
> > 
> > ...should take 10 seconds or so.
> I'm told 0day does *some* suspend/resume testing, but I think it's
> pretty limited, partly because the kinds of machines it primarily
> works on don't really support suspend/resume at all.

currently, we're running suspend test on 1 platform only, with 64 bit
kernel. suspend test will be enabled on more platforms (laptops) in
next two weeks.

I will check why it does not find the first regression introduced by
ca37e57bbe0c ("x86/entry/64: Add missing irqflags tracing to
native_load_gs_index()").

>  I'm also not sure
> just how many of those machines are 32-bit at all..

for this, I suppose it can be reproduced if we use 32-bit kernel and
rootfs, right? Then it's easier to enable this in 0Day.

thanks,
rui
> 
> But I'm adding Zhang Rui to the cc, to see if my recollection is
> right.
> 
> Because you're right, more suspend/resume automated testing would be
> good to have. And yes, people test mainly 64-bit these days.
> 
> Also, I'm not even sure what the 0day rules are for just plain
> mainline. I don't tend to see a lot of breakage reports, even though
> I'd expect to. This came in from the x86 trees (and those do their
> own
> tests too, but probably not suspend/resume either), but it hit my
> tree
> fairly soon after going into the x86 -tip trees.
> 
> Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sun 2017-12-10 13:28:50, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 12:43 PM, Pavel Machek  wrote:
> >
> > For the record... this should fix it. Tested on x60. More tests pending.
> 
> This can't be right.
> 
> At the very least, now the comment is wrong. And the comment does seem
> relevant for 32-bit too:

Well, take a look at orignal patch. I'm reverting 32-bit code to
v4.15-rc1 version, while keeping 64-bit code at v4.15-rc3
version. Yes, my brain hurts from looking at the code :-(.

In the meantime, I did short test on 64-bit machine. No ill effect observed.

Hmm. Aha. Yes, the comment is wrong... as it was in wrong in -rc1.

> > -   fix_processor_context();
> > -
> > /*
> >  * Restore segment registers.  This happens after restoring the GDT
> >  * and LDT, which happen in fix_processor_context().
> 
> Notice? You've moved down the 32-bit fix_processor_context() call to
> after the loadsegment() calls, which smells wrong.

Yeah, I did. There's where it was in v4.15-rc1, and that's what ws
working for me. 

> That said, this *all* smells wrong. Why is there a special
> fix_processor_context() function at all with different 32-bit and
> 64-bit behavior? This code is all written to be maximally confusing.
> 
> I think this could do with some re-org to make it more logical. That
> "some random things done in fix_processor_context(), other random
> things done directly in __restore_processor_state()" makes no sense at
> all to me. There's no logic to what is done where.

I have to agree.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sun 2017-12-10 13:28:50, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 12:43 PM, Pavel Machek  wrote:
> >
> > For the record... this should fix it. Tested on x60. More tests pending.
> 
> This can't be right.
> 
> At the very least, now the comment is wrong. And the comment does seem
> relevant for 32-bit too:

Well, take a look at orignal patch. I'm reverting 32-bit code to
v4.15-rc1 version, while keeping 64-bit code at v4.15-rc3
version. Yes, my brain hurts from looking at the code :-(.

In the meantime, I did short test on 64-bit machine. No ill effect observed.

Hmm. Aha. Yes, the comment is wrong... as it was in wrong in -rc1.

> > -   fix_processor_context();
> > -
> > /*
> >  * Restore segment registers.  This happens after restoring the GDT
> >  * and LDT, which happen in fix_processor_context().
> 
> Notice? You've moved down the 32-bit fix_processor_context() call to
> after the loadsegment() calls, which smells wrong.

Yeah, I did. There's where it was in v4.15-rc1, and that's what ws
working for me. 

> That said, this *all* smells wrong. Why is there a special
> fix_processor_context() function at all with different 32-bit and
> 64-bit behavior? This code is all written to be maximally confusing.
> 
> I think this could do with some re-org to make it more logical. That
> "some random things done in fix_processor_context(), other random
> things done directly in __restore_processor_state()" makes no sense at
> all to me. There's no logic to what is done where.

I have to agree.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 12:43 PM, Pavel Machek  wrote:
>
> For the record... this should fix it. Tested on x60. More tests pending.

This can't be right.

At the very least, now the comment is wrong. And the comment does seem
relevant for 32-bit too:

> -   fix_processor_context();
> -
> /*
>  * Restore segment registers.  This happens after restoring the GDT
>  * and LDT, which happen in fix_processor_context().

Notice? You've moved down the 32-bit fix_processor_context() call to
after the loadsegment() calls, which smells wrong.

That said, this *all* smells wrong. Why is there a special
fix_processor_context() function at all with different 32-bit and
64-bit behavior? This code is all written to be maximally confusing.

I think this could do with some re-org to make it more logical. That
"some random things done in fix_processor_context(), other random
things done directly in __restore_processor_state()" makes no sense at
all to me. There's no logic to what is done where.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 12:43 PM, Pavel Machek  wrote:
>
> For the record... this should fix it. Tested on x60. More tests pending.

This can't be right.

At the very least, now the comment is wrong. And the comment does seem
relevant for 32-bit too:

> -   fix_processor_context();
> -
> /*
>  * Restore segment registers.  This happens after restoring the GDT
>  * and LDT, which happen in fix_processor_context().

Notice? You've moved down the 32-bit fix_processor_context() call to
after the loadsegment() calls, which smells wrong.

That said, this *all* smells wrong. Why is there a special
fix_processor_context() function at all with different 32-bit and
64-bit behavior? This code is all written to be maximally confusing.

I think this could do with some re-org to make it more logical. That
"some random things done in fix_processor_context(), other random
things done directly in __restore_processor_state()" makes no sense at
all to me. There's no logic to what is done where.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sun 2017-12-10 12:30:52, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
> >
> > Confirmed, revert fixes it. You see how it moves fix_processor_context
> > around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
> > machines exist? Aha.
> 
> Yeah, people do.
> 
> Andy?

For the record... this should fix it. Tested on x60. More tests pending.

Signed-off-by: Pavel Machek 


diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 5191de1..d59f05f 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -216,8 +216,8 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
write_cr0(ctxt->cr0);
 
/*
-* now restore the descriptor tables to their proper values
-* ltr is done i fix_processor_context().
+* Now restore the descriptor tables to their proper values
+* ltr is done in fix_processor_context().
 */
 #ifdef CONFIG_X86_32
load_idt(>idt);
@@ -235,8 +235,6 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
wrmsrl(MSR_GS_BASE, ctxt->gs_base);
 #endif
 
-   fix_processor_context();
-
/*
 * Restore segment registers.  This happens after restoring the GDT
 * and LDT, which happen in fix_processor_context().
@@ -252,8 +250,12 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
 */
if (boot_cpu_has(X86_FEATURE_SEP))
enable_sep_cpu();
+
+   fix_processor_context();
 #else
 /* CONFIG_X86_64 */
+   fix_processor_context();
+
asm volatile ("movw %0, %%ds" :: "r" (ctxt->ds));
asm volatile ("movw %0, %%es" :: "r" (ctxt->es));
asm volatile ("movw %0, %%fs" :: "r" (ctxt->fs));


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sun 2017-12-10 12:30:52, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
> >
> > Confirmed, revert fixes it. You see how it moves fix_processor_context
> > around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
> > machines exist? Aha.
> 
> Yeah, people do.
> 
> Andy?

For the record... this should fix it. Tested on x60. More tests pending.

Signed-off-by: Pavel Machek 


diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 5191de1..d59f05f 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -216,8 +216,8 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
write_cr0(ctxt->cr0);
 
/*
-* now restore the descriptor tables to their proper values
-* ltr is done i fix_processor_context().
+* Now restore the descriptor tables to their proper values
+* ltr is done in fix_processor_context().
 */
 #ifdef CONFIG_X86_32
load_idt(>idt);
@@ -235,8 +235,6 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
wrmsrl(MSR_GS_BASE, ctxt->gs_base);
 #endif
 
-   fix_processor_context();
-
/*
 * Restore segment registers.  This happens after restoring the GDT
 * and LDT, which happen in fix_processor_context().
@@ -252,8 +250,12 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
 */
if (boot_cpu_has(X86_FEATURE_SEP))
enable_sep_cpu();
+
+   fix_processor_context();
 #else
 /* CONFIG_X86_64 */
+   fix_processor_context();
+
asm volatile ("movw %0, %%ds" :: "r" (ctxt->ds));
asm volatile ("movw %0, %%es" :: "r" (ctxt->es));
asm volatile ("movw %0, %%fs" :: "r" (ctxt->fs));


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
>
> Confirmed, revert fixes it. You see how it moves fix_processor_context
> around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
> machines exist? Aha.

Yeah, people do.

Andy?

> Which brings me to .. various people do automated testing of
> kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit for
> boot and suspend would be very nice. The last item is not hard, either:
>
> sudo rtcwake -l -m mem -s 5
>
> ...should take 10 seconds or so.

I'm told 0day does *some* suspend/resume testing, but I think it's
pretty limited, partly because the kinds of machines it primarily
works on don't really support suspend/resume at all. I'm also not sure
just how many of those machines are 32-bit at all..

But I'm adding Zhang Rui to the cc, to see if my recollection is right.

Because you're right, more suspend/resume automated testing would be
good to have. And yes, people test mainly 64-bit these days.

Also, I'm not even sure what the 0day rules are for just plain
mainline. I don't tend to see a lot of breakage reports, even though
I'd expect to. This came in from the x86 trees (and those do their own
tests too, but probably not suspend/resume either), but it hit my tree
fairly soon after going into the x86 -tip trees.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 10:56 AM, Pavel Machek  wrote:
>
> Confirmed, revert fixes it. You see how it moves fix_processor_context
> around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
> machines exist? Aha.

Yeah, people do.

Andy?

> Which brings me to .. various people do automated testing of
> kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit for
> boot and suspend would be very nice. The last item is not hard, either:
>
> sudo rtcwake -l -m mem -s 5
>
> ...should take 10 seconds or so.

I'm told 0day does *some* suspend/resume testing, but I think it's
pretty limited, partly because the kinds of machines it primarily
works on don't really support suspend/resume at all. I'm also not sure
just how many of those machines are 32-bit at all..

But I'm adding Zhang Rui to the cc, to see if my recollection is right.

Because you're right, more suspend/resume automated testing would be
good to have. And yes, people test mainly 64-bit these days.

Also, I'm not even sure what the 0day rules are for just plain
mainline. I don't tend to see a lot of breakage reports, even though
I'd expect to. This came in from the x86 trees (and those do their own
tests too, but probably not suspend/resume either), but it hit my tree
fairly soon after going into the x86 -tip trees.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Michal Hocko]

On Thu 07-12-17 08:55:08, Michal Hocko wrote:
> On Wed 06-12-17 13:14:52, Michal Hocko wrote:
> > On Mon 04-12-17 14:36:20, Linus Torvalds wrote:
> > > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki  
> > > wrote:
> > > >
> > > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > > systems I have tested, so it is probably safe to assume it to be
> > > > broken everywhere.
> > > 
> > > Oh, it's definitely not broken everywhere, because I use it myself,
> > > and was traveling last week due to my mom's bday.
> > > 
> > > HOWEVER.
> > > 
> > > Some of the x86 work seems to have broken it for some configurations.
> > > In particular, do you have a big "everything enabled" kernel config -
> > > particularly lockdep and irqflags tracing enabled?
> > > 
> > > Andy has a patch, but it hasn't made it to me yet (probably because
> > > the x86 people are very busy with the kaiser work):
> > > 
> > > https://lkml.org/lkml/2017/11/30/546
> > > 
> > > (also note his follow-up "fix the commit message" note, but that one
> > > doesn't actually affect the code itself).
> > 
> > merging tip/x86/urgent on top of your tree fixed this problem for me,
> > but I am seeing something else
> > [  131.711412] ACPI: Preparing to enter system sleep state S3
> > [  131.755328] ACPI: EC: event blocked
> > [  131.755328] ACPI: EC: EC stopped
> > [  131.755328] PM: Saving platform NVS memory
> > [  131.755344] Disabling non-boot CPUs ...
> > [  131.779330] IRQ 124: no longer affine to CPU1
> > [  131.780334] smpboot: CPU 1 is now offline
> > [  131.804465] smpboot: CPU 2 is now offline
> > [  131.827291] IRQ 122: no longer affine to CPU3
> > [  131.827292] IRQ 123: no longer affine to CPU3
> > [  131.828293] smpboot: CPU 3 is now offline
> > [  131.830991] ACPI: Low-level resume complete
> > [  131.831092] ACPI: EC: EC started
> > [  131.831093] PM: Restoring platform NVS memory
> > [  131.831864] do_IRQ: 0.55 No irq handler for vector
> > [  131.831884] Enabling non-boot CPUs ...
> > [  131.831909] x86: Booting SMP configuration:
> > [  131.831910] smpboot: Booting Node 0 Processor 1 APIC 0x2
> > [  131.832913]  cache: parent cpu1 should not be sleeping
> > [  131.833058] CPU1 is up
> > [  131.833067] smpboot: Booting Node 0 Processor 2 APIC 0x1
> > [  131.833864]  cache: parent cpu2 should not be sleeping
> > [  131.833983] CPU2 is up
> > [  131.833995] smpboot: Booting Node 0 Processor 3 APIC 0x3
> > [  131.834776]  cache: parent cpu3 should not be sleeping
> > [  131.834923] CPU3 is up
> > 
> > "No irq handler" part looks a bit scary (maybe related to lost affinity
> > messages?) but the following messages look quite as well. Is this
> > something known? The system seems to be up and running without any
> > visible issues.
> 
> Hmm, there is still something bad going on during resume. My laptop
> haven't woken up from s2ram this morning. The screen was powered on
> but the system hasn't come up.

It's been few days and I haven't seen this problem again. And I am doing
s2ram all the time...
-- 
Michal Hocko
SUSE Labs

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Michal Hocko]

On Thu 07-12-17 08:55:08, Michal Hocko wrote:
> On Wed 06-12-17 13:14:52, Michal Hocko wrote:
> > On Mon 04-12-17 14:36:20, Linus Torvalds wrote:
> > > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki  
> > > wrote:
> > > >
> > > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > > systems I have tested, so it is probably safe to assume it to be
> > > > broken everywhere.
> > > 
> > > Oh, it's definitely not broken everywhere, because I use it myself,
> > > and was traveling last week due to my mom's bday.
> > > 
> > > HOWEVER.
> > > 
> > > Some of the x86 work seems to have broken it for some configurations.
> > > In particular, do you have a big "everything enabled" kernel config -
> > > particularly lockdep and irqflags tracing enabled?
> > > 
> > > Andy has a patch, but it hasn't made it to me yet (probably because
> > > the x86 people are very busy with the kaiser work):
> > > 
> > > https://lkml.org/lkml/2017/11/30/546
> > > 
> > > (also note his follow-up "fix the commit message" note, but that one
> > > doesn't actually affect the code itself).
> > 
> > merging tip/x86/urgent on top of your tree fixed this problem for me,
> > but I am seeing something else
> > [  131.711412] ACPI: Preparing to enter system sleep state S3
> > [  131.755328] ACPI: EC: event blocked
> > [  131.755328] ACPI: EC: EC stopped
> > [  131.755328] PM: Saving platform NVS memory
> > [  131.755344] Disabling non-boot CPUs ...
> > [  131.779330] IRQ 124: no longer affine to CPU1
> > [  131.780334] smpboot: CPU 1 is now offline
> > [  131.804465] smpboot: CPU 2 is now offline
> > [  131.827291] IRQ 122: no longer affine to CPU3
> > [  131.827292] IRQ 123: no longer affine to CPU3
> > [  131.828293] smpboot: CPU 3 is now offline
> > [  131.830991] ACPI: Low-level resume complete
> > [  131.831092] ACPI: EC: EC started
> > [  131.831093] PM: Restoring platform NVS memory
> > [  131.831864] do_IRQ: 0.55 No irq handler for vector
> > [  131.831884] Enabling non-boot CPUs ...
> > [  131.831909] x86: Booting SMP configuration:
> > [  131.831910] smpboot: Booting Node 0 Processor 1 APIC 0x2
> > [  131.832913]  cache: parent cpu1 should not be sleeping
> > [  131.833058] CPU1 is up
> > [  131.833067] smpboot: Booting Node 0 Processor 2 APIC 0x1
> > [  131.833864]  cache: parent cpu2 should not be sleeping
> > [  131.833983] CPU2 is up
> > [  131.833995] smpboot: Booting Node 0 Processor 3 APIC 0x3
> > [  131.834776]  cache: parent cpu3 should not be sleeping
> > [  131.834923] CPU3 is up
> > 
> > "No irq handler" part looks a bit scary (maybe related to lost affinity
> > messages?) but the following messages look quite as well. Is this
> > something known? The system seems to be up and running without any
> > visible issues.
> 
> Hmm, there is still something bad going on during resume. My laptop
> haven't woken up from s2ram this morning. The screen was powered on
> but the system hasn't come up.

It's been few days and I haven't seen this problem again. And I am doing
s2ram all the time...
-- 
Michal Hocko
SUSE Labs

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sun 2017-12-10 08:37:56, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 8:23 AM, Pavel Machek  wrote:
> >
> > Can you do something about html emails? Quoting them doesn't work too well.
> 
> Yeah, and they don't show up onlkml either because of rules. I try to
> avoid them, but have been more on mobile for various reasons lately
> than usual. That should be over and done with after today, though.
> 
> >> Any chance to bisect it? This doesn't sound like the other problem
> >> we had.
> >
> > Let me try...
> >
> > v4.15-rc2: suspends/resumes ok.
> > 4ded3be: hangs on resume.
> >
> > Given that between those, there was supposed "fix" for suspend, I
> > believe I should try reverting that one first. .. if someone can tell
> > me commit id, that would help.
> 
> The fix in there should be 5b06bbcfc2c6 ("x86/power: Fix some ordering
> bugs in __restore_processor_context()") so you can certainly see if it
> works before that (or just reverting it).

Revert is easier.

> But there are also a few other x86 low-level things there, and that
> fix really looks very safe, so I'd almost expect something else to
> have triggered your problem. There's less than 500 commits in that
> range you have, so a few bisections should narrow it down a lot.

No, that commit does _look_ pretty suspect to me...

Confirmed, revert fixes it. You see how it moves fix_processor_context
around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
machines exist? Aha.

Which brings me to .. various people do automated testing of
kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit for
boot and suspend would be very nice. The last item is not hard, either:

sudo rtcwake -l -m mem -s 5

...should take 10 seconds or so. 

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sun 2017-12-10 08:37:56, Linus Torvalds wrote:
> On Sun, Dec 10, 2017 at 8:23 AM, Pavel Machek  wrote:
> >
> > Can you do something about html emails? Quoting them doesn't work too well.
> 
> Yeah, and they don't show up onlkml either because of rules. I try to
> avoid them, but have been more on mobile for various reasons lately
> than usual. That should be over and done with after today, though.
> 
> >> Any chance to bisect it? This doesn't sound like the other problem
> >> we had.
> >
> > Let me try...
> >
> > v4.15-rc2: suspends/resumes ok.
> > 4ded3be: hangs on resume.
> >
> > Given that between those, there was supposed "fix" for suspend, I
> > believe I should try reverting that one first. .. if someone can tell
> > me commit id, that would help.
> 
> The fix in there should be 5b06bbcfc2c6 ("x86/power: Fix some ordering
> bugs in __restore_processor_context()") so you can certainly see if it
> works before that (or just reverting it).

Revert is easier.

> But there are also a few other x86 low-level things there, and that
> fix really looks very safe, so I'd almost expect something else to
> have triggered your problem. There's less than 500 commits in that
> range you have, so a few bisections should narrow it down a lot.

No, that commit does _look_ pretty suspect to me...

Confirmed, revert fixes it. You see how it moves fix_processor_context
around #ifdef CONFIG_X86_32 block? And how people forget 32-bit
machines exist? Aha.

Which brings me to .. various people do automated testing of
kernel. Testing 32-bit kernel for boot, and both 32-bit and 64-bit for
boot and suspend would be very nice. The last item is not hard, either:

sudo rtcwake -l -m mem -s 5

...should take 10 seconds or so. 

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 8:23 AM, Pavel Machek  wrote:
>
> Can you do something about html emails? Quoting them doesn't work too well.

Yeah, and they don't show up onlkml either because of rules. I try to
avoid them, but have been more on mobile for various reasons lately
than usual. That should be over and done with after today, though.

>> Any chance to bisect it? This doesn't sound like the other problem
>> we had.
>
> Let me try...
>
> v4.15-rc2: suspends/resumes ok.
> 4ded3be: hangs on resume.
>
> Given that between those, there was supposed "fix" for suspend, I
> believe I should try reverting that one first. .. if someone can tell
> me commit id, that would help.

The fix in there should be 5b06bbcfc2c6 ("x86/power: Fix some ordering
bugs in __restore_processor_context()") so you can certainly see if it
works before that (or just reverting it).

But there are also a few other x86 low-level things there, and that
fix really looks very safe, so I'd almost expect something else to
have triggered your problem. There's less than 500 commits in that
range you have, so a few bisections should narrow it down a lot.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Linus Torvalds]

On Sun, Dec 10, 2017 at 8:23 AM, Pavel Machek  wrote:
>
> Can you do something about html emails? Quoting them doesn't work too well.

Yeah, and they don't show up onlkml either because of rules. I try to
avoid them, but have been more on mobile for various reasons lately
than usual. That should be over and done with after today, though.

>> Any chance to bisect it? This doesn't sound like the other problem
>> we had.
>
> Let me try...
>
> v4.15-rc2: suspends/resumes ok.
> 4ded3be: hangs on resume.
>
> Given that between those, there was supposed "fix" for suspend, I
> believe I should try reverting that one first. .. if someone can tell
> me commit id, that would help.

The fix in there should be 5b06bbcfc2c6 ("x86/power: Fix some ordering
bugs in __restore_processor_context()") so you can certainly see if it
works before that (or just reverting it).

But there are also a few other x86 low-level things there, and that
fix really looks very safe, so I'd almost expect something else to
have triggered your problem. There's less than 500 commits in that
range you have, so a few bisections should narrow it down a lot.

Linus

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sat 2017-12-09 14:47:41, Linus Torvalds wrote:
> On Dec 9, 2017 14:01, "Pavel Machek"  wrote:
> 
> 
> Strange. I was at 4.15-rc1, and suspend worked there (thinkpad x60,
> 32-bit). It was broken in -next. I updated to current mainline (
> 4ded3bec65a07343258ed8fd9d46483f032d866f ) and suspend is broken.
> 

Can you do something about html emails? Quoting them doesn't work too well.

> Any chance to bisect it? This doesn't sound like the other problem
> we had.

Let me try...

v4.15-rc2: suspends/resumes ok.
4ded3be: hangs on resume.

Given that between those, there was supposed "fix" for suspend, I
believe I should try reverting that one first. .. if someone can tell
me commit id, that would help.

And yes, if everything else fails, I can probably bisect.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sat 2017-12-09 14:47:41, Linus Torvalds wrote:
> On Dec 9, 2017 14:01, "Pavel Machek"  wrote:
> 
> 
> Strange. I was at 4.15-rc1, and suspend worked there (thinkpad x60,
> 32-bit). It was broken in -next. I updated to current mainline (
> 4ded3bec65a07343258ed8fd9d46483f032d866f ) and suspend is broken.
> 

Can you do something about html emails? Quoting them doesn't work too well.

> Any chance to bisect it? This doesn't sound like the other problem
> we had.

Let me try...

v4.15-rc2: suspends/resumes ok.
4ded3be: hangs on resume.

Given that between those, there was supposed "fix" for suspend, I
believe I should try reverting that one first. .. if someone can tell
me commit id, that would help.

And yes, if everything else fails, I can probably bisect.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

Hi!

On Sat 2017-12-09 10:55:14, Linus Torvalds wrote:
> On Dec 9, 2017 02:33, "Pavel Machek"  wrote:
> 
> > I believe I have the issue here, too (-next on thinkpad x60). Which
> > patch is expected to fix it? Let me try recent -next...
> 
> 
> It should be fixed in mainline. I don't know if next has picked it
> up yet.

Strange. I was at 4.15-rc1, and suspend worked there (thinkpad x60,
32-bit). It was broken in -next. I updated to current mainline (
4ded3bec65a07343258ed8fd9d46483f032d866f ) and suspend is broken.

It suspends ok, I press Fn button to make it resume, fans spin up but
moon LED is still lit.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

Hi!

On Sat 2017-12-09 10:55:14, Linus Torvalds wrote:
> On Dec 9, 2017 02:33, "Pavel Machek"  wrote:
> 
> > I believe I have the issue here, too (-next on thinkpad x60). Which
> > patch is expected to fix it? Let me try recent -next...
> 
> 
> It should be fixed in mainline. I don't know if next has picked it
> up yet.

Strange. I was at 4.15-rc1, and suspend worked there (thinkpad x60,
32-bit). It was broken in -next. I updated to current mainline (
4ded3bec65a07343258ed8fd9d46483f032d866f ) and suspend is broken.

It suspends ok, I press Fn button to make it resume, fans spin up but
moon LED is still lit.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sat 2017-12-09 11:33:25, Pavel Machek wrote:
> On Tue 2017-12-05 01:25:55, Rafael J. Wysocki wrote:
> > On Monday, December 4, 2017 11:41:06 PM CET Rafael J. Wysocki wrote:
> > > On Monday, December 4, 2017 11:38:54 PM CET Thomas Gleixner wrote:
> > > > On Mon, 4 Dec 2017, Linus Torvalds wrote:
> > > > 
> > > > > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki 
> > > > >  wrote:
> > > > > >
> > > > > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > > > > systems I have tested, so it is probably safe to assume it to be
> > > > > > broken everywhere.
> > > > > 
> > > > > Oh, it's definitely not broken everywhere, because I use it myself,
> > > > > and was traveling last week due to my mom's bday.
> > > > > 
> > > > > HOWEVER.
> > > > > 
> > > > > Some of the x86 work seems to have broken it for some configurations.
> > > > > In particular, do you have a big "everything enabled" kernel config -
> > > > > particularly lockdep and irqflags tracing enabled?
> > > > > 
> > > > > Andy has a patch, but it hasn't made it to me yet (probably because
> > > > > the x86 people are very busy with the kaiser work):
> > > 
> > > This definitely fixes the problem at least on one of the affected 
> > > machines.
> > 
> > I can confirm that the Andy's patch fixes it on all systems that had this
> > issue here.
> 
> I believe I have the issue here, too (-next on thinkpad x60). Which
> patch is expected to fix it? Let me try recent -next...

Still there AFAICT.
Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Sat 2017-12-09 11:33:25, Pavel Machek wrote:
> On Tue 2017-12-05 01:25:55, Rafael J. Wysocki wrote:
> > On Monday, December 4, 2017 11:41:06 PM CET Rafael J. Wysocki wrote:
> > > On Monday, December 4, 2017 11:38:54 PM CET Thomas Gleixner wrote:
> > > > On Mon, 4 Dec 2017, Linus Torvalds wrote:
> > > > 
> > > > > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki 
> > > > >  wrote:
> > > > > >
> > > > > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > > > > systems I have tested, so it is probably safe to assume it to be
> > > > > > broken everywhere.
> > > > > 
> > > > > Oh, it's definitely not broken everywhere, because I use it myself,
> > > > > and was traveling last week due to my mom's bday.
> > > > > 
> > > > > HOWEVER.
> > > > > 
> > > > > Some of the x86 work seems to have broken it for some configurations.
> > > > > In particular, do you have a big "everything enabled" kernel config -
> > > > > particularly lockdep and irqflags tracing enabled?
> > > > > 
> > > > > Andy has a patch, but it hasn't made it to me yet (probably because
> > > > > the x86 people are very busy with the kaiser work):
> > > 
> > > This definitely fixes the problem at least on one of the affected 
> > > machines.
> > 
> > I can confirm that the Andy's patch fixes it on all systems that had this
> > issue here.
> 
> I believe I have the issue here, too (-next on thinkpad x60). Which
> patch is expected to fix it? Let me try recent -next...

Still there AFAICT.
Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Tue 2017-12-05 01:25:55, Rafael J. Wysocki wrote:
> On Monday, December 4, 2017 11:41:06 PM CET Rafael J. Wysocki wrote:
> > On Monday, December 4, 2017 11:38:54 PM CET Thomas Gleixner wrote:
> > > On Mon, 4 Dec 2017, Linus Torvalds wrote:
> > > 
> > > > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki  
> > > > wrote:
> > > > >
> > > > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > > > systems I have tested, so it is probably safe to assume it to be
> > > > > broken everywhere.
> > > > 
> > > > Oh, it's definitely not broken everywhere, because I use it myself,
> > > > and was traveling last week due to my mom's bday.
> > > > 
> > > > HOWEVER.
> > > > 
> > > > Some of the x86 work seems to have broken it for some configurations.
> > > > In particular, do you have a big "everything enabled" kernel config -
> > > > particularly lockdep and irqflags tracing enabled?
> > > > 
> > > > Andy has a patch, but it hasn't made it to me yet (probably because
> > > > the x86 people are very busy with the kaiser work):
> > 
> > This definitely fixes the problem at least on one of the affected machines.
> 
> I can confirm that the Andy's patch fixes it on all systems that had this
> issue here.

I believe I have the issue here, too (-next on thinkpad x60). Which
patch is expected to fix it? Let me try recent -next...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Pavel Machek]

On Tue 2017-12-05 01:25:55, Rafael J. Wysocki wrote:
> On Monday, December 4, 2017 11:41:06 PM CET Rafael J. Wysocki wrote:
> > On Monday, December 4, 2017 11:38:54 PM CET Thomas Gleixner wrote:
> > > On Mon, 4 Dec 2017, Linus Torvalds wrote:
> > > 
> > > > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki  
> > > > wrote:
> > > > >
> > > > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > > > systems I have tested, so it is probably safe to assume it to be
> > > > > broken everywhere.
> > > > 
> > > > Oh, it's definitely not broken everywhere, because I use it myself,
> > > > and was traveling last week due to my mom's bday.
> > > > 
> > > > HOWEVER.
> > > > 
> > > > Some of the x86 work seems to have broken it for some configurations.
> > > > In particular, do you have a big "everything enabled" kernel config -
> > > > particularly lockdep and irqflags tracing enabled?
> > > > 
> > > > Andy has a patch, but it hasn't made it to me yet (probably because
> > > > the x86 people are very busy with the kaiser work):
> > 
> > This definitely fixes the problem at least on one of the affected machines.
> 
> I can confirm that the Andy's patch fixes it on all systems that had this
> issue here.

I believe I have the issue here, too (-next on thinkpad x60). Which
patch is expected to fix it? Let me try recent -next...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 7 Dec 2017, Maarten Lankhorst wrote:
> Op 06-12-17 om 15:15 schreef Thomas Gleixner:
> > On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
> >> Op 06-12-17 om 13:46 schreef Thomas Gleixner:
> >>> On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
>  Op 06-12-17 om 13:15 schreef Michal Hocko:
> > "No irq handler" part looks a bit scary (maybe related to lost affinity
> > messages?) but the following messages look quite as well. Is this
> > something known? The system seems to be up and running without any
> > visible issues.
>  Another reproducer for 
>  https://bugzilla.kernel.org/show_bug.cgi?id=198033 ?
>  Symptoms are similar..
> >>> Well, the spurious interrupt is one thing, but you obviously lose
> >>> interrupts for some reason.
> >>>
> >>> Did you ever manage to get the data out which I asked for?
> >>>
> >>> Thanks,
> >>>
> >>>   tglx
> >>>
> >> Yes, sent this out about an hour ago
> >>
> >> https://lkml.org/lkml/2017/12/6/215
> > Weird. Did not reach me
> >
> But do you have any idea?

Can you please provide the full trace, dmesg and the full output of
.../debug/irq/... ?

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Thomas Gleixner]

On Thu, 7 Dec 2017, Maarten Lankhorst wrote:
> Op 06-12-17 om 15:15 schreef Thomas Gleixner:
> > On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
> >> Op 06-12-17 om 13:46 schreef Thomas Gleixner:
> >>> On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
>  Op 06-12-17 om 13:15 schreef Michal Hocko:
> > "No irq handler" part looks a bit scary (maybe related to lost affinity
> > messages?) but the following messages look quite as well. Is this
> > something known? The system seems to be up and running without any
> > visible issues.
>  Another reproducer for 
>  https://bugzilla.kernel.org/show_bug.cgi?id=198033 ?
>  Symptoms are similar..
> >>> Well, the spurious interrupt is one thing, but you obviously lose
> >>> interrupts for some reason.
> >>>
> >>> Did you ever manage to get the data out which I asked for?
> >>>
> >>> Thanks,
> >>>
> >>>   tglx
> >>>
> >> Yes, sent this out about an hour ago
> >>
> >> https://lkml.org/lkml/2017/12/6/215
> > Weird. Did not reach me
> >
> But do you have any idea?

Can you please provide the full trace, dmesg and the full output of
.../debug/irq/... ?

Thanks,

tglx

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Maarten Lankhorst]

Op 06-12-17 om 15:15 schreef Thomas Gleixner:
> On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
>> Op 06-12-17 om 13:46 schreef Thomas Gleixner:
>>> On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
 Op 06-12-17 om 13:15 schreef Michal Hocko:
> "No irq handler" part looks a bit scary (maybe related to lost affinity
> messages?) but the following messages look quite as well. Is this
> something known? The system seems to be up and running without any
> visible issues.
 Another reproducer for https://bugzilla.kernel.org/show_bug.cgi?id=198033 ?
 Symptoms are similar..
>>> Well, the spurious interrupt is one thing, but you obviously lose
>>> interrupts for some reason.
>>>
>>> Did you ever manage to get the data out which I asked for?
>>>
>>> Thanks,
>>>
>>> tglx
>>>
>> Yes, sent this out about an hour ago
>>
>> https://lkml.org/lkml/2017/12/6/215
> Weird. Did not reach me
>
But do you have any idea?

~Maarten

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Maarten Lankhorst]

Op 06-12-17 om 15:15 schreef Thomas Gleixner:
> On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
>> Op 06-12-17 om 13:46 schreef Thomas Gleixner:
>>> On Wed, 6 Dec 2017, Maarten Lankhorst wrote:
 Op 06-12-17 om 13:15 schreef Michal Hocko:
> "No irq handler" part looks a bit scary (maybe related to lost affinity
> messages?) but the following messages look quite as well. Is this
> something known? The system seems to be up and running without any
> visible issues.
 Another reproducer for https://bugzilla.kernel.org/show_bug.cgi?id=198033 ?
 Symptoms are similar..
>>> Well, the spurious interrupt is one thing, but you obviously lose
>>> interrupts for some reason.
>>>
>>> Did you ever manage to get the data out which I asked for?
>>>
>>> Thanks,
>>>
>>> tglx
>>>
>> Yes, sent this out about an hour ago
>>
>> https://lkml.org/lkml/2017/12/6/215
> Weird. Did not reach me
>
But do you have any idea?

~Maarten

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Michal Hocko]

On Wed 06-12-17 13:14:52, Michal Hocko wrote:
> On Mon 04-12-17 14:36:20, Linus Torvalds wrote:
> > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki  
> > wrote:
> > >
> > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > systems I have tested, so it is probably safe to assume it to be
> > > broken everywhere.
> > 
> > Oh, it's definitely not broken everywhere, because I use it myself,
> > and was traveling last week due to my mom's bday.
> > 
> > HOWEVER.
> > 
> > Some of the x86 work seems to have broken it for some configurations.
> > In particular, do you have a big "everything enabled" kernel config -
> > particularly lockdep and irqflags tracing enabled?
> > 
> > Andy has a patch, but it hasn't made it to me yet (probably because
> > the x86 people are very busy with the kaiser work):
> > 
> > https://lkml.org/lkml/2017/11/30/546
> > 
> > (also note his follow-up "fix the commit message" note, but that one
> > doesn't actually affect the code itself).
> 
> merging tip/x86/urgent on top of your tree fixed this problem for me,
> but I am seeing something else
> [  131.711412] ACPI: Preparing to enter system sleep state S3
> [  131.755328] ACPI: EC: event blocked
> [  131.755328] ACPI: EC: EC stopped
> [  131.755328] PM: Saving platform NVS memory
> [  131.755344] Disabling non-boot CPUs ...
> [  131.779330] IRQ 124: no longer affine to CPU1
> [  131.780334] smpboot: CPU 1 is now offline
> [  131.804465] smpboot: CPU 2 is now offline
> [  131.827291] IRQ 122: no longer affine to CPU3
> [  131.827292] IRQ 123: no longer affine to CPU3
> [  131.828293] smpboot: CPU 3 is now offline
> [  131.830991] ACPI: Low-level resume complete
> [  131.831092] ACPI: EC: EC started
> [  131.831093] PM: Restoring platform NVS memory
> [  131.831864] do_IRQ: 0.55 No irq handler for vector
> [  131.831884] Enabling non-boot CPUs ...
> [  131.831909] x86: Booting SMP configuration:
> [  131.831910] smpboot: Booting Node 0 Processor 1 APIC 0x2
> [  131.832913]  cache: parent cpu1 should not be sleeping
> [  131.833058] CPU1 is up
> [  131.833067] smpboot: Booting Node 0 Processor 2 APIC 0x1
> [  131.833864]  cache: parent cpu2 should not be sleeping
> [  131.833983] CPU2 is up
> [  131.833995] smpboot: Booting Node 0 Processor 3 APIC 0x3
> [  131.834776]  cache: parent cpu3 should not be sleeping
> [  131.834923] CPU3 is up
> 
> "No irq handler" part looks a bit scary (maybe related to lost affinity
> messages?) but the following messages look quite as well. Is this
> something known? The system seems to be up and running without any
> visible issues.

Hmm, there is still something bad going on during resume. My laptop
haven't woken up from s2ram this morning. The screen was powered on
but the system hasn't come up.

The last thing that made it into the kernel log on fs is this

Dec  6 19:32:29 tiehlicka kernel: [21898.084685] PM: suspend entry (deep)

which won't tell us much I suspect. I've tried dozen s2ram cycles and it
hasn't reproduced so it smells like a timing issue.
-- 
Michal Hocko
SUSE Labs

Re: Linux 4.15-rc2: Regression in resume from ACPI S3

[Michal Hocko]

On Wed 06-12-17 13:14:52, Michal Hocko wrote:
> On Mon 04-12-17 14:36:20, Linus Torvalds wrote:
> > On Mon, Dec 4, 2017 at 2:25 PM, Rafael J. Wysocki  
> > wrote:
> > >
> > > So far, resume from suspend-to-RAM (ACPI S3) is broken on all of the
> > > systems I have tested, so it is probably safe to assume it to be
> > > broken everywhere.
> > 
> > Oh, it's definitely not broken everywhere, because I use it myself,
> > and was traveling last week due to my mom's bday.
> > 
> > HOWEVER.
> > 
> > Some of the x86 work seems to have broken it for some configurations.
> > In particular, do you have a big "everything enabled" kernel config -
> > particularly lockdep and irqflags tracing enabled?
> > 
> > Andy has a patch, but it hasn't made it to me yet (probably because
> > the x86 people are very busy with the kaiser work):
> > 
> > https://lkml.org/lkml/2017/11/30/546
> > 
> > (also note his follow-up "fix the commit message" note, but that one
> > doesn't actually affect the code itself).
> 
> merging tip/x86/urgent on top of your tree fixed this problem for me,
> but I am seeing something else
> [  131.711412] ACPI: Preparing to enter system sleep state S3
> [  131.755328] ACPI: EC: event blocked
> [  131.755328] ACPI: EC: EC stopped
> [  131.755328] PM: Saving platform NVS memory
> [  131.755344] Disabling non-boot CPUs ...
> [  131.779330] IRQ 124: no longer affine to CPU1
> [  131.780334] smpboot: CPU 1 is now offline
> [  131.804465] smpboot: CPU 2 is now offline
> [  131.827291] IRQ 122: no longer affine to CPU3
> [  131.827292] IRQ 123: no longer affine to CPU3
> [  131.828293] smpboot: CPU 3 is now offline
> [  131.830991] ACPI: Low-level resume complete
> [  131.831092] ACPI: EC: EC started
> [  131.831093] PM: Restoring platform NVS memory
> [  131.831864] do_IRQ: 0.55 No irq handler for vector
> [  131.831884] Enabling non-boot CPUs ...
> [  131.831909] x86: Booting SMP configuration:
> [  131.831910] smpboot: Booting Node 0 Processor 1 APIC 0x2
> [  131.832913]  cache: parent cpu1 should not be sleeping
> [  131.833058] CPU1 is up
> [  131.833067] smpboot: Booting Node 0 Processor 2 APIC 0x1
> [  131.833864]  cache: parent cpu2 should not be sleeping
> [  131.833983] CPU2 is up
> [  131.833995] smpboot: Booting Node 0 Processor 3 APIC 0x3
> [  131.834776]  cache: parent cpu3 should not be sleeping
> [  131.834923] CPU3 is up
> 
> "No irq handler" part looks a bit scary (maybe related to lost affinity
> messages?) but the following messages look quite as well. Is this
> something known? The system seems to be up and running without any
> visible issues.

Hmm, there is still something bad going on during resume. My laptop
haven't woken up from s2ram this morning. The screen was powered on
but the system hasn't come up.

The last thing that made it into the kernel log on fs is this

Dec  6 19:32:29 tiehlicka kernel: [21898.084685] PM: suspend entry (deep)

which won't tell us much I suspect. I've tried dozen s2ram cycles and it
hasn't reproduced so it smells like a timing issue.
-- 
Michal Hocko
SUSE Labs