Lag after resume culprit found

[Andriy Gapon]

https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa
x86 cpususpend_handler: call wbinvd after setting suspend state bits

That's very interesting and surprising.
That commit changes something that happens before suspend, it should not have
any effect on the system state after resume.

Does anyone have a theory of what could be wrong?

How to test (i915kms)
Start X with glxgears
Confirm running stable at 60 fps
suspend/resume (S3)
glxgears is now fluctuating between 10-40 fps.

[Johannes Lundberg]

https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f93317

1eaa6e1cfa

x86 cpususpend_handler: call wbinvd after setting suspend state bits

That's very interesting and surprising.
That commit changes something that happens before suspend, it should not have
any effect on the system state after resume.
Does anyone have a theory of what could be wrong?

Nope but moving
CPU_CLR_ATOMIC(cpu, &suspended_cpus);
back to the end of that scope fixes it.

How to test (i915kms)
Start X with glxgears
Confirm running stable at 60 fps
suspend/resume (S3)
glxgears is now fluctuating between 10-40 fps.

--
Andriy Gapon

[Johannes Lundberg]

Post by Johannes Lundberg

https://github.com/freebsd/freebsd/commit/66f063557f257baa9c

8aeab9f933171eaa6e1cfa

x86 cpususpend_handler: call wbinvd after setting suspend state bits

That's very interesting and surprising.
That commit changes something that happens before suspend, it should not have
any effect on the system state after resume.
Does anyone have a theory of what could be wrong?

Nope but moving
CPU_CLR_ATOMIC(cpu, &suspended_cpus);
back to the end of that scope fixes it.

I did some further testing.
Calling
CPU_CLR_ATOMIC(cpu, &suspended_cpus);
before
pmap_init_pat();
is what "breaks" resume.

Is this Intel only or this it happen on AMD as well (which this patch was
intended for)?

Post by Johannes Lundberg

How to test (i915kms)
Start X with glxgears
Confirm running stable at 60 fps
suspend/resume (S3)
glxgears is now fluctuating between 10-40 fps.

--
Andriy Gapon

[Andriy Gapon]

https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa

<https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa>

x86 cpususpend_handler: call wbinvd after setting suspend state bits

That's very interesting and surprising.
That commit changes something that happens before suspend, it should not
have
any effect on the system state after resume.
Does anyone have a theory of what could be wrong?
Nope but moving
        CPU_CLR_ATOMIC(cpu, &suspended_cpus);
back to the end of that scope fixes it.
 
I did some further testing.
Calling
CPU_CLR_ATOMIC(cpu, &suspended_cpus);
before
pmap_init_pat();
 is what "breaks" resume.
Is this Intel only or this it happen on AMD as well (which this patch was
intended for)?

Not sure about the PAT part, but fpuresume/npxresume would affect all platforms.
It's a bit puzzling that doing PAT manipulations on one AP while another AP is
being brought up is problematic. Probably there is something that I am missing.

Thank you very much again for zeroing in on it.

How to test (i915kms)
Start X with glxgears
Confirm running stable at 60 fps
suspend/resume (S3)
glxgears is now fluctuating between 10-40 fps.

[Konstantin Belousov]

Post by Andriy Gapon

https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa

<https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa>

x86 cpususpend_handler: call wbinvd after setting suspend state bits

That's very interesting and surprising.
That commit changes something that happens before suspend, it should not
have
any effect on the system state after resume.
Does anyone have a theory of what could be wrong?
Nope but moving
        CPU_CLR_ATOMIC(cpu, &suspended_cpus);
back to the end of that scope fixes it.
 
I did some further testing.
Calling
CPU_CLR_ATOMIC(cpu, &suspended_cpus);
before
pmap_init_pat();
 is what "breaks" resume.
Is this Intel only or this it happen on AMD as well (which this patch was
intended for)?

Not sure about the PAT part, but fpuresume/npxresume would affect all platforms.
It's a bit puzzling that doing PAT manipulations on one AP while another AP is
being brought up is problematic. Probably there is something that I am missing.

Manipulating PAT might affect the cache consistency, since contradicting
caching attributes are applied to the line of the suspended_cpus variable
which is already cached. It might be not the variable itself that causes
the final mis-operation, but some other data sharing the line.

[Andriy Gapon]

https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa

<https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa>

x86 cpususpend_handler: call wbinvd after setting suspend state bits

That's very interesting and surprising.
That commit changes something that happens before suspend, it should not have
any effect on the system state after resume.
Does anyone have a theory of what could be wrong?
Nope but moving
        CPU_CLR_ATOMIC(cpu, &suspended_cpus);
back to the end of that scope fixes it.

That's interesting.
Thank you for testing it!
And let me think about it.

How to test (i915kms)
Start X with glxgears
Confirm running stable at 60 fps
suspend/resume (S3)
glxgears is now fluctuating between 10-40 fps.

[Andriy Gapon]

Post by Andriy Gapon

https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa

<https://github.com/freebsd/freebsd/commit/66f063557f257baa9c8aeab9f933171eaa6e1cfa>

x86 cpususpend_handler: call wbinvd after setting suspend state bits

That's very interesting and surprising.
That commit changes something that happens before suspend, it should not have
any effect on the system state after resume.
Does anyone have a theory of what could be wrong?
Nope but moving
        CPU_CLR_ATOMIC(cpu, &suspended_cpus);
back to the end of that scope fixes it.

That's interesting.
Thank you for testing it!
And let me think about it.

Oh, I am stupid.
I intended that operation to be right after the CPU is done with restoring its
saved context. Which means that it has to be after fpuresume/npxresume block.
Could you please re-test with CPU_CLR_ATOMIC(cpu, &suspended_cpus) at that position?
And my apologies.

Post by Andriy Gapon

How to test (i915kms)
Start X with glxgears
Confirm running stable at 60 fps
suspend/resume (S3)
glxgears is now fluctuating between 10-40 fps.