Page MenuHomeFreeBSD

amd64: Initialize IPI scoreboard earlier
ClosedPublic

Authored by kib on Sep 27 2022, 8:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 8 2024, 11:56 AM
Unknown Object (File)
Dec 24 2023, 9:08 PM
Unknown Object (File)
Dec 20 2023, 8:12 AM
Unknown Object (File)
Sep 18 2023, 11:15 PM
Unknown Object (File)
Sep 18 2023, 11:15 PM
Unknown Object (File)
Sep 18 2023, 11:15 PM
Unknown Object (File)
Sep 18 2023, 11:12 PM
Unknown Object (File)
Sep 3 2023, 1:37 PM
Subscribers

Details

Summary
Scoreboard is needed a moment when smp_started == true.  If some kernel
daemon thread is started before scoreboard is inited, and does some pmap
operation that requires TLB maintanence, which races with SMP startup,
we might dereference NULL invl_scoreboard.  This is particularly easy
to trigger when EARLY_AP_STARTUP is not defined.  I do not think we start
any kernel thread before SMP startup when EARLY_AP_STARTUP is defined.

Reported by:    glebius

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib requested review of this revision.Sep 27 2022, 8:04 PM

You gave me the patch in December 2021 and I was running with it for a while. I think all the time until SMR inpcb branch has been merged to main. After that I lost the patch in my rebases, so has been running without it last half a year. Didn't see the panic, though. But as you remember the panic is quite rare. I think the patch should go in as long as logically it is correct. I'm no expert in the area at all.

This revision is now accepted and ready to land.Sep 27 2022, 11:32 PM

I do not think we start any kernel thread before SMP startup when EARLY_AP_STARTUP is defined.

FWIW, the audit subsystem starts one at SI_SUB_AUDIT. There could be others.

sys/amd64/amd64/mp_machdep.c
582

start_aps() also uses SI_SUB_SMP/SI_ORDER_FIRST. Can't we just initialize the scoreboard at SI_SUB_SMP - 1 to solve the problem?

kib marked an inline comment as done.

Use SI_SUB_SMP - 1.

This revision now requires review to proceed.Sep 28 2022, 12:44 PM
This revision is now accepted and ready to land.Sep 28 2022, 1:03 PM
sys/amd64/amd64/mp_machdep.c
582

Does not have to be _FIRST.

sys/amd64/amd64/mp_machdep.c
582

I considered changing it to _ANY, but really do not see a difference.
Do you insist? Or do you mean something else?

sys/amd64/amd64/mp_machdep.c
582

There's no difference, but whenever I see _FIRST is used, I assume there is some specific reason for it. _ANY is effectively a default. So, it is just a bit confusing IMO. I don't insist though.

This revision was automatically updated to reflect the committed changes.