amd64: ifunc-ify pmap_invalidate_XXX() TLB invalidation functions.
ClosedPublic
Actions

Authored by kib on Sep 16 2018, 10:00 PM.

Details

Reviewers

alc
markj
jhb

Commits

rS338862: Convert x86 TLB top-level invalidation functions to ifuncs.

Summary

This is a continuation of D17181.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 19733

Event Timeline

kib created this revision.Sep 16 2018, 10:00 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 16 2018, 10:00 PM

Harbormaster completed remote builds in B19611: Diff 48089.Sep 16 2018, 10:00 PM

This diff is generated from the branch where pmap_activate_sw() work was done. I will update the review after the context switch chunks will be committed.

Distilled patch after pmap_activate_sw() changes were committed.

Harbormaster completed remote builds in B19624: Diff 48122.Sep 17 2018, 3:56 PM

markj accepted this revision.Sep 17 2018, 4:10 PM

This revision is now accepted and ready to land.Sep 17 2018, 4:10 PM

Peter, could you, please, give some run time to this patch ? It is only relevant for amd64, but in two modes, with pcid enabled and with vm.pmap.pcid_enabled=0. In both cases, some threading loads and some mmap tests are enough.

In D17184#367884, @kib wrote:

Peter, could you, please, give some run time to this patch ? It is only relevant for amd64, but in two modes, with pcid enabled and with vm.pmap.pcid_enabled=0. In both cases, some threading loads and some mmap tests are enough.

Did that + a buildworld / installworld. No problems seen.

alc accepted this revision.Sep 21 2018, 4:53 PM

alc added inline comments.

sys/amd64/amd64/pmap.c
1722	I would suggest refactoring these sentences: "Because pm_pcid is recalculated on a context switch, we must disable switching. Otherwise, we might use a stale value below."
1726	Arguably, the blank line isn't needed here because the comment above is relevant to this entire block.
1749–1756	This has been deindented by one level. It might be worth reflowing the paragraph.
1805–1806	This is an aside. I wonder how often these days we wake up a halted processor to perform a shootdown on the kernel address space.
2024–2025	Is there any reason for this particular ordering of these statements? Swapping their order, would mean that the variable mask did not have to be live across a function call, and thus the compiler would not have to use a callee-saves register.

kib marked 5 inline comments as done.Sep 21 2018, 5:33 PM

kib added inline comments.

sys/amd64/amd64/pmap.c
1749–1756	My emacs does not want to change the formatting.
1805–1806	Yes. I read some descriptions (but not the code) that Linux does not IPI CPUs which are in deep sleep, say C2 or C3. Instead, a flag is set and CPU does TLB invalidation on wakeup.
2024–2025	It might be not, but I want this change to be tested separately, if done. Also, at least for symmetry, other top level invalidation functions can get the same structure by the cost of one more if().

Reword comment, strip at eol.

This revision now requires review to proceed.Sep 21 2018, 5:35 PM

Harbormaster completed remote builds in B19733: Diff 48323.Sep 21 2018, 5:35 PM

alc accepted this revision.Sep 21 2018, 5:42 PM

This revision is now accepted and ready to land.Sep 21 2018, 5:42 PM

alc added inline comments.Sep 21 2018, 5:45 PM

sys/amd64/amd64/pmap.c
1805–1806	Given recent changes, we could start using the kernel pmap cpu mask to track which processors actually require shootdown. I know that some of the sleep states actually flush the TLB based on experimental observation, but I don't know what is documented in this regard.

Closed by commit rS338862: Convert x86 TLB top-level invalidation functions to ifuncs. (authored by kib). · Explain WhySep 21 2018, 5:53 PM

This revision was automatically updated to reflect the committed changes.

kib marked an inline comment as done.Sep 21 2018, 6:01 PM

kib added inline comments.

sys/amd64/amd64/pmap.c
1805–1806	You mean vmspace0.pmap, or kernel pmap ? Anyway, documentation states that C3 writes back and invalidates CPU caches, but I do not remember any mention of the TLB flush.

kib mentioned this in D17277: Further reorganize pmap_invalidate TLB code..Sep 21 2018, 8:48 PM

kernel pmap. Specifically, I would propose clearing the processor's bit in the kernel pmap when it halts. That said, we would still need to have a separate flag indicating when the processor needs to take action when it wakes. (Depending on the documentation of the deeper sleep states maybe that action is a NOP.)

Revision Contents
Changeset List

Path

Size

sys/

amd64/

pmap.c

365 lines

Diff 48323

View Options

amd64: ifunc-ify pmap_invalidate_XXX() TLB invalidation functions.ClosedPublicActions