Page MenuHomeFreeBSD

Add missed barrier for pm_gen/pm_active interaction.
ClosedPublic

Authored by kib on May 21 2018, 2:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 2, 9:26 AM
Unknown Object (File)
Jan 21 2024, 11:30 PM
Unknown Object (File)
Jan 11 2024, 12:01 AM
Unknown Object (File)
Dec 20 2023, 1:08 AM
Unknown Object (File)
Dec 16 2023, 3:35 PM
Unknown Object (File)
Sep 28 2023, 7:26 PM
Unknown Object (File)
Sep 27 2023, 10:36 PM
Unknown Object (File)
Sep 27 2023, 7:38 AM
Subscribers

Details

Summary

When we issue shootdown IPIs, we first assign zero to pm_gens to indicate the need to flush on the next context switch in case our IPI misses the context, next we read pm_active. On context switch we set our bit in pm_active, then we read pm_gen. It is crucial that both threads see the memory in the program order, otherwise invalidation thread might read pm_active bit as zero and the context switching thread might read pm_gen as zero.

IA32 allows CPU for both reads to see zero. We must use the barriers between write and read. The pm_active bit set is already locked, so only the invalidation functions need it.

I never saw it in real life, or at least I do not have a good reproduction case. I found this during code inspection when hunting for the Xen TLB issue reported by cperciva.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Can you please add a comment to the code explaining the need for the barrier. The comment in the second and third instances could simply refer the reader to the comment in the first instance.

This revision is now accepted and ready to land.May 21 2018, 4:08 PM
This revision now requires review to proceed.May 21 2018, 4:18 PM
markj added inline comments.
sys/amd64/amd64/pmap.c
1726

"reads of"

1729

"simultaneously observe a non-zero ..."?

1731

"such an outcome"

1732

"a locked operation"

This revision is now accepted and ready to land.May 21 2018, 4:41 PM
kib marked 4 inline comments as done.

Mark' grammar fixes.

This revision now requires review to proceed.May 21 2018, 4:59 PM
sys/amd64/amd64/pmap.c
1726

I'm sorry, I was wrong here. Should be, "the stores to pm_gen and the read of the pm_active mask."

This revision is now accepted and ready to land.May 21 2018, 6:18 PM
sys/amd64/amd64/pmap.c
1807

"See the comment in ...

1880

"See the comment in ...

This revision was automatically updated to reflect the committed changes.