amd64 pmap: block on turnstile for lock-less DI

[kib]

Summary

Port the code to block on turnstile to lock-less delayed invalidation, instead of yielding. Yield might cause tight loop due to priority inversion.

Since it is impossible to avoid race between block and wake-up, arm 1-tick callout to wakeup when thread blocks itself.

Reported and tested by: mjg

[markj]

sys/amd64/amd64/pmap.c

1000 ↗

(On Diff #58624)

Why do you recheck the loop condition here?

[alc]

I would suggest that you add a comment to the code explaining why the callout is necessary.

[kib]

In D20636#446616, @alc wrote:

I would suggest that you add a comment to the code explaining why the callout is necessary.

I will do it when I will be back at my workstation (in about a week).

sys/amd64/amd64/pmap.c

1000 ↗

(On Diff #58624)

I do it to claim that the only race is due to the lost wakeup. We have in the thread finishing DI:

store generation (atomic, works as seq consistent fence)
load pmap_invl_waiters

Without re-check, the order in the waiting thread is

load generation
increment pmap_invl_waiters (seq cst)

which is reverse order to what is needed to make the increment fence to work against the store generation fence. With the re-check, the order becomes

increment pmap_invl_waiters (seq cst)
load generation

You might argue that the callout would close this race anyway, but callout doing the real work means that the thread lost up to 1 tick of the wall time sitting blocked instead of making progress. So leaving callout use only to wakeup race proper is useful.

[markj]

sys/amd64/amd64/pmap.c

1000 ↗

(On Diff #58624)

Thanks. I think this should be explained in a comment.

[kib]

Add comments explaining the callout use and recheck.

[alc]

sys/amd64/amd64/pmap.c

999 ↗

(On Diff #58906)

should be "page's"

1000 ↗

(On Diff #58906)

I would say, "... below the current thread's number. Prepare to block so that we do not waste CPU cycles or worse, suffer livelock."

1004 ↗

(On Diff #58906)

"... impossible to block without ..."

1007 ↗

(On Diff #58906)

"... incrementing ..."

1014 ↗

(On Diff #58906)

"... thread's invalidation ..." (Note the spelling correction to "invlidation".)

[kib]

Language corrections.

[alc]

sys/amd64/amd64/pmap.c

1008 ↗

(On Diff #58921)

Verb tense: "... which will unblock us if we lose ..."

1014 ↗

(On Diff #58921)

"Re-check the current ..."

[kib]

Two more language fixes.