Linux 4.15-rc2

It's Sunday, but a few hours earlier than usual, since I'm on the east coast, three hours ahead of my normal release schedule.

It's a slightly bigger rc2 than I would have wished for, but this early in the release process I don't worry about it. The appended shortlog gives the details, it's fixes all over the place - architectures, drivers, filesystems, networking, core kernel.

One thing I'll point out is that I'm trying to get some kernel ASLR leaks plugged, and as part of that we now hash any pointers printed by "%p' by default. That won't affect a lot of people, but where it is a debugging problem (rather than leaking interesting kernel pointers), we will have to fix things up.

It can be a small annoyance, but the alternatives (trying to actually find all the cases where we might be leaking) were worse. But let's see if anybody even notices - a lot of the pointer printouts are stale debug information from when some driver was originally written, and aren't actually really interesting.

There will probably be some more leak fixes during this rc process, we'll see how that all sorts out.

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.

Quoted 4 lines · Rafael J. Wysocki

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):

Quoted 9 lines · Linus Torvalds

Quoted 4 lines · Rafael J. Wysocki

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.

Quoted 8 lines

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):

Picking it up right now.

Quoted 13 lines · CET Thomas Gleixner

Quoted 11 lines · Linus Torvalds

Quoted 4 lines · Rafael J. Wysocki

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.

Quoted 6 lines

Quoted 6 lines

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.

Quoted 18 lines · CET Rafael J. Wysocki

Quoted 17 lines · CET Thomas Gleixner

Quoted 15 lines · Linus Torvalds

Quoted 4 lines · Rafael J. Wysocki

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?

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

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.

Quoted 22 lines · Rafael J. Wysocki

Quoted 21 lines · CET Rafael J. Wysocki

Quoted 19 lines · CET Thomas Gleixner

Quoted 17 lines · Linus Torvalds

Quoted 4 lines · Rafael J. Wysocki

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):

Quoted 4 lines

Quoted 1 line

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...

Quoted 2 lines · Pavel Machek

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.

On Dec 9, 2017 14:01, "Pavel Machek" <[email protected]> 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.

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

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

Quoted 2 lines

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.

Quoted 2 lines · Pavel Machek

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.

Quoted 8 lines

Quoted 2 lines

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.

Quoted 3 lines

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.

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.

Quoted 4 lines · Pavel Machek

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.

Quoted 2 lines · Pavel Machek

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:

arch/x86/power/cpu.c static void notrace __restore_processor_state(struct saved_context *ctxt) · line 240

Quoted 5 lines

arch/x86/power/cpu.c +0 -2

-       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.

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.

Quoted 4 lines · Linus Torvalds

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?

Quoted 5 lines

- 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, ...).

Quoted 8 lines · Andy Lutomirski

Quoted 5 lines

- 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,

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

Quoted 2 lines · Andy Lutomirski

(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

Quoted 1 line · Linus Torvalds

On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski <[email protected]> wrote:

Like this?

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

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

Quoted 7 lines · Andy Lutomirski

Quoted 1 line · Linus Torvalds

On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski <[email protected]> wrote:

Like this?

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

Quoted 3 lines

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 <[email protected]>

Quoted 12 lines · Andy Lutomirski

Quoted 3 lines · Linus Torvalds

On Tue, Dec 12, 2017 at 10:05 AM, Andy Lutomirski <[email protected]> wrote:

Like this?

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

Quoted 3 lines

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 <[email protected]>

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

While we are at it, fix a comment.

Signed-off-by Pavel Machek <[email protected]>

Fixes 5b06bbcfc2c621da3009da8decb7511500c293ed

arch/x86/power/cpu.c +14 -7

index 5191de1..af7b613 100644
@@ -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(&ctxt->idt);

@@ -235,13 +235,11 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)
   wrmsrl(MSR_GS_BASE, ctxt->gs_base);
 #endif
 
-    fix_processor_context();
-
+#ifdef CONFIG_X86_32
   /*
-     * Restore segment registers.  This happens after restoring the GDT
-     * and LDT, which happen in fix_processor_context().
+     * Restore segment registers.
    */
-#ifdef CONFIG_X86_32
+
   loadsegment(es, ctxt->es);
   loadsegment(fs, ctxt->fs);
   loadsegment(gs, ctxt->gs);

@@ -252,8 +250,17 @@ 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 */
+    /*
+     * Restore segment registers.  This happens after restoring the GDT
+     * and LDT, which happen in fix_processor_context().
+     */
+    
+    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));

Signature · 3 lines

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

Quoted 8 lines · Pavel Machek

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

Can you explain what was wrong with the reordering? Your patch certainly *looks* incorrect.

I'm guessing that the real issue is that 32-bit needs %fs restored
early for TLS.

Quoted 57 lines

While we are at it, fix a comment.

Signed-off-by Pavel Machek <[email protected]>

Fixes 5b06bbcfc2c621da3009da8decb7511500c293ed

arch/x86/power/cpu.c +14 -7

index 5191de1..af7b613 100644
@@ -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(&ctxt->idt);

@@ -235,13 +235,11 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)
   wrmsrl(MSR_GS_BASE, ctxt->gs_base);
 #endif
 
-    fix_processor_context();
-
+#ifdef CONFIG_X86_32
   /*
-     * Restore segment registers.  This happens after restoring the GDT
-     * and LDT, which happen in fix_processor_context().
+     * Restore segment registers.
    */
-#ifdef CONFIG_X86_32
+
   loadsegment(es, ctxt->es);
   loadsegment(fs, ctxt->fs);
   loadsegment(gs, ctxt->gs);

@@ -252,8 +250,17 @@ 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 */
+    /*
+     * Restore segment registers.  This happens after restoring the GDT
+     * and LDT, which happen in fix_processor_context().
+     */
+    
+    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));

Quoted 11 lines · Andy Lutomirski

Quoted 8 lines · Pavel Machek

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

Can you explain what was wrong with the reordering? Your patch certainly *looks* incorrect.

No, I can't, sorry.

Quoted 2 lines

I'm guessing that the real issue is that 32-bit needs %fs restored
early for TLS.

Maybe. I can test patches...

I don't think it would be good idea to revert 5b06bbcfc2c621da3009da8decb7511500c293ed, but since it introduced regression in -rc2, I believe we should fix the regression now, and then we can try to provide cleaner solution.

Quoted 9 lines · Pavel Machek

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

Can you explain what was wrong with the reordering? Your patch certainly *looks* incorrect.

I'm guessing that the real issue is that 32-bit needs %fs restored early for TLS.

Quoted 10 lines · CET Andy Lutomirski

Quoted 9 lines · Pavel Machek

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

Quoted 3 lines

Can you explain what was wrong with the reordering? Your patch certainly *looks* incorrect.

I'm guessing that the real issue is that 32-bit needs %fs restored early for TLS.

I *think* you are right.

Quoted 10 lines · CET Andy Lutomirski

Quoted 9 lines · Pavel Machek

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

Quoted 3 lines

Can you explain what was wrong with the reordering? Your patch certainly *looks* incorrect.

I'm guessing that the real issue is that 32-bit needs %fs restored early for TLS.

I *think* you are right.

Anyway, that should be easy enough to verify.

Pavel, can you please check if the below change works too?

---
  arch/x86/power/cpu.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 2 lines · CET Andy Lutomirski

I'm guessing that the real issue is that 32-bit needs %fs restored early for TLS.

I *think* you are right.

Anyway, that should be easy enough to verify.

Pavel, can you please check if the below change works too?

Quoted 9 lines · Rafael J. Wysocki

Quoted 2 lines · CET Andy Lutomirski

I'm guessing that the real issue is that 32-bit needs %fs restored early for TLS.

I *think* you are right.

Anyway, that should be easy enough to verify.

Pavel, can you please check if the below change works too?

So Jarkko confirmed this works for him, but the more I look at this crap, the less I like it.

Why do we save fs/ds/es/ss at all on x86-32? Don't they all have fixed values in the kernel, with %fs being __KERNEL_PERCPU, and the others being __USER_DS?

Nothing else can possibly be valid, as far as I can tell.

I think we actually leave the user-space percpu segment in %gs (or the stack canary base), so that one we should actually save/restore, but I'm getting the feeling that we should just reset the other segment registers to known values on 32-bit.

Also, why does the 32-bit code do

        loadsegment(es, ctxt->es);

but the 64-bit code does

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

And look at that confusion between MSR_GS_BASE and MSR_KERNEL_GS_BASE all within the 64-bit case.

In particular, note how we reload the %gs segment in between the two - wouldn't that mess with the currently active gs base if %gs can be non-zero?

Christ, what a mess.

So I think that whole sequence is garbage. It has been written as some kind of "save and restore registers", but that's not what it really then does - or what it should do.

It should make sure to restore a sane kernel state, not some random register state.

And the 32-bit and 64-bit code really should strive to be at least _sanely_ different, not this randomly and insanely different mess.

But yes, Rafael's patch looks like the minimal one-liner. But I think we should do the %gs load early too for the 32-bit stack canary case, kind of like we need to do %fs for percpu base.

I'll try to get to this in a day or so -- is that okay? Or should we do some trivial fix/revert and fix it for real next time around?

Quoted 3 lines

But yes, Rafael's patch looks like the minimal one-liner. But I think we should do the %gs load early too for the 32-bit stack canary case, kind of like we need to do %fs for percpu base.

I'll try to get to this in a day or so -- is that okay? Or should we do some trivial fix/revert and fix it for real next time around?

But given this is already "regression fix for x86-64 caused regression on x86-32", I really believe we should merge trivial fix now, and do the cleanups / nicer fixes sometime later?

Quoted 4 lines · Pavel Machek

But given this is already "regression fix for x86-64 caused regression on x86-32", I really believe we should merge trivial fix now, and do the cleanups / nicer fixes sometime later?

The fix patch was already posted, but in another thread (confusingly with _almost_ the same subject: "Re: Linux 4.15-rc2: Regression in resume from ACPI S3").

Here:

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

but it hasn't actually gotten to me yet (I think Luto made a new version where he split it up into three smaller patches, and I'm assuming it's in one of the -tip trees heading for me asap).

All the x86 people have been insanely busy with the crazy page table isolation patches, that probably is slowing down things. But I was expecting to get the -tip tree pulls tomorrow (.. because.. Friday. It's my busiest day of the week because everybody wants to get their work out before the weekend).

Quoted 4 lines · Pavel Machek

But given this is already "regression fix for x86-64 caused regression on x86-32", I really believe we should merge trivial fix now, and do the cleanups / nicer fixes sometime later?

The fix patch was already posted, but in another thread (confusingly with _almost_ the same subject: "Re: Linux 4.15-rc2: Regression in resume from ACPI S3").

Here:

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

but it hasn't actually gotten to me yet (I think Luto made a new version where he split it up into three smaller patches, and I'm assuming it's in one of the -tip trees heading for me asap).

Quoted 10 lines · Pavel Machek

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

Can you explain what was wrong with the reordering? Your patch certainly *looks* incorrect.

I'm guessing that the real issue is that 32-bit needs %fs restored early for TLS.

Quoted 11 lines · Andy Lutomirski

Quoted 10 lines · Pavel Machek

After 4.15-rc2, suspend stopped working on Thinkpad X60. 5b06bbc (unintentionally?) reordered stuff with respect to fix_processor_context() on 32-bit and 64-bit. We undo that change on 32-bit.

Quoted 3 lines

Can you explain what was wrong with the reordering? Your patch certainly *looks* incorrect.

I'm guessing that the real issue is that 32-bit needs %fs restored early for TLS.

Does some early percpu primitive need GS as well perhaps?

Might be safest to restore both FS and GS early.

Quoted 6 lines · Pavel Machek

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?

Quoted 7 lines

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:

Quoted 3 lines

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

Quoted 18 lines · Linus Torvalds

Quoted 6 lines · Pavel Machek

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?

Quoted 7 lines

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:

Quoted 6 lines

Quoted 3 lines

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()").

Quoted 2 lines

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

Quoted 16 lines

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

Quoted 4 lines

Quoted 1 line

...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.

Quoted 2 lines

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.

Quoted 4 lines · Rafael J. Wysocki

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.

Quoted 17 lines · Michal Hocko

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

Hmm, that's really odd.

Quoted 4 lines

[  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

This is an old one.

Quoted 8 lines

[  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

Quoted 4 lines

"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.

I assume it's due to the affinity break, just that we don't know right now on which CPU that do_IRQ() message triggered. I assume it's CPU0 because the others are offline already, but ....

I'll think about it how we can figure out what's going on.

Quoted 13 lines · Rafael J. Wysocki

Quoted 3 lines

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?

Quoted 7 lines

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.

Op 06-12-17 om 13:15 schreef Michal Hocko:

Quoted 5 lines

"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..

Quoted 7 lines

Op 06-12-17 om 13:15 schreef Michal Hocko:

Quoted 4 lines

"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

Op 06-12-17 om 13:46 schreef Thomas Gleixner:

Quoted 13 lines · Maarten Lankhorst

Quoted 7 lines

Op 06-12-17 om 13:15 schreef Michal Hocko:

Quoted 4 lines

"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?

Quoted 4 lines

Thanks,

tglx

Yes, sent this out about an hour ago

https://lkml.org/lkml/2017/12/6/215

Quoted 14 lines

Op 06-12-17 om 13:46 schreef Thomas Gleixner:

Quoted 13 lines · Maarten Lankhorst

Quoted 7 lines

Op 06-12-17 om 13:15 schreef Michal Hocko:

Quoted 4 lines

"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?

Quoted 7 lines

Quoted 4 lines

Thanks,

tglx

Yes, sent this out about an hour ago

https://lkml.org/lkml/2017/12/6/215

Weird. Did not reach me

Op 06-12-17 om 15:15 schreef Thomas Gleixner:

Quoted 17 lines · Maarten Lankhorst

Quoted 16 lines

Op 06-12-17 om 13:46 schreef Thomas Gleixner:

Quoted 15 lines · Maarten Lankhorst

Quoted 7 lines

Op 06-12-17 om 13:15 schreef Michal Hocko:

Quoted 4 lines

"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,

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

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?

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

[+cc linux-pci, linux-pm]

Quoted 22 lines · 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;
   }

Quoted 2 lines

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.

Quoted 3 lines · Thomas Gleixner

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?

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner

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.

Quoted 15 lines · Thomas Gleixner

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner

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.

Quoted 4 lines

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.

Quoted 26 lines · Thomas Gleixner

Quoted 15 lines · Thomas Gleixner

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner

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.

Quoted 4 lines

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.

Quoted 1 line

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.

Quoted 21 lines · Thomas Gleixner

Quoted 19 lines · Thomas Gleixner

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner

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.

Quoted 6 lines

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.

Quoted 22 lines · Thomas Gleixner

Quoted 21 lines · Thomas Gleixner

Quoted 19 lines · Thomas Gleixner

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner

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.

Quoted 6 lines

Quoted 6 lines

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: 0000:00:02.0 00000000fee0100c 0000412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   ....
   device_pm_callback_start: i915 0000:00:02.0, parent: pci0000: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: 0000:00:02.0 00000000fee0100c 0000412a  <-- Set the new affinity
   device_pm_callback_end: i915 0000:00:02.0, err=0

Hibernate:

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   ....
   device_pm_callback_start: i915 0000:00:02.0, parent: pci0000:00, noirq bus [thaw]
   pci_pm_thaw_noirq <-dpm_run_callback
   __pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
   __pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
   device_pm_callback_end: i915 0000: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 that unearthes the issue. I'm handing that over to the PM folks to look at. I got lost in that maze of callbacks.

Quoted 7 lines · CET Thomas Gleixner

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:

Quoted 11 lines

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   ....
   device_pm_callback_start: i915 0000:00:02.0, parent: pci0000: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: 0000:00:02.0 00000000fee0100c 0000412a  <-- Set the new affinity
   device_pm_callback_end: i915 0000: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.

Quoted 11 lines

Hibernate:

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   ....
   device_pm_callback_start: i915 0000:00:02.0, parent: pci0000:00, noirq bus [thaw]
   pci_pm_thaw_noirq <-dpm_run_callback
   __pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
   __pci_write_msi_msg: Not written  <--- Device is not in PCI_D0
   device_pm_callback_end: i915 0000: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.

Quoted 35 lines · Rafael J. Wysocki

Quoted 7 lines · CET Thomas Gleixner

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:

Quoted 11 lines

   irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10
   __pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
   __pci_write_msi_msg: Not written <- Device not in PCI_D0
   ....
   device_pm_callback_start: i915 0000:00:02.0, parent: pci0000: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: 0000:00:02.0 00000000fee0100c 0000412a  <-- Set the new affinity
   device_pm_callback_end: i915 0000: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.

Quoted 11 lines

Hibernate:

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

Quoted 5 lines

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.

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
===================================================================

drivers/pci/pci-driver.c +6 -1

@@ -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)

Quoted 11 lines · Rafael J. Wysocki

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: 0000:00:02.0 00000000fee0100c 0000412a __pci_write_msi_msg: Not written
...
device_pm_callback_start: i915 0000:00:02.0, parent: pci0000:00, noirq bus [thaw] pci_pm_thaw_noirq <-dpm_run_callback
__pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
device_pm_callback_end: i915 0000:00:02.0, err=0
...
resume_irqs: Resume 125
...
irq_handler_entry: irq=125 name=i915

Quoted 14 lines · Rafael J. Wysocki

Quoted 11 lines

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:

Quoted 12 lines

irq_migrate_all_off_this_cpu: Mask 125 pci_msi_mask_irq+0x0/0x10 __pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a __pci_write_msi_msg: Not written
...
device_pm_callback_start: i915 0000:00:02.0, parent: pci0000:00, noirq bus [thaw] pci_pm_thaw_noirq <-dpm_run_callback
__pci_write_msi_msg: 0000:00:02.0 00000000fee0100c 0000412a
device_pm_callback_end: i915 0000: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

From: Rafael J. Wysocki <[email protected]>

It is incorrect to call pci_restore_state() for devices in low-power states (D1-D3), as that involves the restoration of MSI setup which requires MMIO to be operational and that is only the case in D0.

However, pci_pm_thaw_noirq() may do that if the driver's "freeze" callbacks put the device into a low-power state, so fix it by making it force devices into D0 via pci_set_power_state() instead of trying to "update" their power state which is pointless.

Fixes e60514bd4485 (PCI/PM: Restore the status of PCI devices across hibernation)

Cc 4.13+ <[email protected]> # 4.13+

Reported-by Thomas Gleixner <[email protected]>

Reported-by Maarten Lankhorst <[email protected]>

Tested-by Thomas Gleixner <[email protected]>

Tested-by Maarten Lankhorst <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

---

The bug is not as old as I thought, actually.

Yes, we did the pci_update_current_state() in pci_pm_thaw_noirq() forever, but it started to be problematic in 4.13, when we started to call pci_restore_state() in addition to it to fix another issue.

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

Index: linux-pm/drivers/pci/pci-driver.c
===================================================================

drivers/pci/pci-driver.c +6 -1

@@ -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)

From: Rafael J. Wysocki <[email protected]>

It is incorrect to call pci_restore_state() for devices in low-power states (D1-D3), as that involves the restoration of MSI setup which requires MMIO to be operational and that is only the case in D0.

However, pci_pm_thaw_noirq() may do that if the driver's "freeze" callbacks put the device into a low-power state, so fix it by making it force devices into D0 via pci_set_power_state() instead of trying to "update" their power state which is pointless.

Fixes e60514bd4485 (PCI/PM: Restore the status of PCI devices across hibernation)

Cc 4.13+ <[email protected]> # 4.13+

Reported-by Thomas Gleixner <[email protected]>

Reported-by Maarten Lankhorst <[email protected]>

Tested-by Thomas Gleixner <[email protected]>

Tested-by Maarten Lankhorst <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

---

The bug is not as old as I thought, actually.

Yes, we did the pci_update_current_state() in pci_pm_thaw_noirq() forever, but it started to be problematic in 4.13, when we started to call pci_restore_state() in addition to it to fix another issue.

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

Index: linux-pm/drivers/pci/pci-driver.c
===================================================================

drivers/pci/pci-driver.c +6 -1

@@ -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)

Quoted 11 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

It is incorrect to call pci_restore_state() for devices in low-power states (D1-D3), as that involves the restoration of MSI setup which requires MMIO to be operational and that is only the case in D0.

However, pci_pm_thaw_noirq() may do that if the driver's "freeze" callbacks put the device into a low-power state, so fix it by making it force devices into D0 via pci_set_power_state() instead of trying to "update" their power state which is pointless.

Quoted 7 lines

Fixes e60514bd4485 (PCI/PM: Restore the status of PCI devices across hibernation)

Cc 4.13+ <[email protected]> # 4.13+

Reported-by Thomas Gleixner <[email protected]>

Reported-by Maarten Lankhorst <[email protected]>

Tested-by Thomas Gleixner <[email protected]>

Tested-by Maarten Lankhorst <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

Acked-by Bjorn Helgaas <[email protected]>

Let me know if you want me to take this. I don't have anything currently queued up that touches pci-driver.c, so I'm happy if you take it yourself.

Quoted 31 lines

---

The bug is not as old as I thought, actually.

Yes, we did the pci_update_current_state() in pci_pm_thaw_noirq() forever, but it started to be problematic in 4.13, when we started to call pci_restore_state() in addition to it to fix another issue.

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

Index: linux-pm/drivers/pci/pci-driver.c
===================================================================

drivers/pci/pci-driver.c +6 -1

@@ -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)

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.

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.

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

On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner <[email protected]> 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:

Quoted 3 lines

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

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.

Quoted 5 lines · Thomas Gleixner

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.

Quoted 18 lines · Linus Torvalds

On Thu, Dec 14, 2017 at 3:54 AM, Thomas Gleixner <[email protected]> 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:

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner

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.

Quoted 1 line

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.

Quoted 20 lines · CET Thomas Gleixner

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Thomas Gleixner

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.

Quoted 7 lines

Quoted 1 line

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).

Quoted 15 lines · Linus Torvalds

Quoted 4 lines · Rafael J. Wysocki

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?

Quoted 7 lines

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.

Quoted 53 lines · Michal Hocko

Quoted 15 lines · Linus Torvalds

Quoted 4 lines · Rafael J. Wysocki

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?

Quoted 7 lines

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

Quoted 4 lines

"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.

Linus Torvalds (6):

      Rename superblock flags (MS_xyz -> SB_xyz)