cvs commit: src/sys/amd64/amd64 local_apic.c
src/sys/i386/acpica madt.c src/sys/i386/i386 local_apic.c
src/sys/kern subr_smp.c src/sys/sun4v/mdesc mdesc_init.c
jhb at freebsd.org
Fri Sep 21 12:27:30 PDT 2007
On Tuesday 11 September 2007 06:54:09 pm Attilio Rao wrote:
> attilio 2007-09-11 22:54:09 UTC
> FreeBSD src repository
> Modified files:
> sys/amd64/amd64 local_apic.c
> sys/i386/acpica madt.c
> sys/i386/i386 local_apic.c
> sys/kern subr_smp.c
> sys/sun4v/mdesc mdesc_init.c
> This is a follow-up, cleaning-up commit about recent changes involving
> topology foo functions.
> Working at the patch for topology problems in ia32/amd64 evicted some
> problems regarding functions ordering in the SI_SUB_CPU family of
> SYSINIT'ed subsystems.
> In order to avoid problems with new modified to involved functions, a
> correct ordering is not semantically specified for SI_SUB_CPU functions
> (for a larger view of the issue please visit:
> http://lists.freebsd.org/pipermail/freebsd-current/2007-July/075409.html )
Could you clarify exactly what ordering this does? AFAICT, nothing in
cpu_startup() requires the APIC to be up and running. If there were a
dependency, then that would be something for the MD architecture code to fix.
IIUC, the problem was that you had 3 sysinit functions like this:
A - SI_ORDER_FIRST, cpu_startup()
B - SI_ORDER_FIRST, apic_init()
C - SI_ORDER_SECOND, mp_start()
Now mp_start() requires both A and B to have run on x86. What Peter did
originally was to move B to SI_ORDER_SECOND because he found that B needed A
to run. That broke because C could end up running before B. However, the
actual patch that went into CVS left A and B as is and instead made them
independent of each other so B no longer depends on A. So, I don't think
you've solved an actual problem.
Conceptually the way SI_SUB_CPU worked before was that the MD code had
SI_ORDER_FIRST to get ready before it was asked to start SMP at
SI_ORDER_SECOND. Now the MD code gets SI_ORDER_FIRST and _SECOND even though
it doesn't need it. You should at least put the x86 apic routines back to
SI_ORDER_FIRST becuase they do _not_ depend on cpu_startup().
The madt.c change in this commit is plain wrong however and should be
reverted. If it was correct it would have needed to be done in amd64's
madt.c as well.
The sun4v change is bogus as well as mdesc_init() doesn't depend on
cpu_startup() on sun4v at all, and it doesn't matter what order they run in.
Basically, I think at the least you should revert all the MD changes, and the
change to subr_smp.c is basically a NOP.
More information about the cvs-all