On Saturday, November 25, 2017 11:41:05 PM Nathan Whitehorn wrote:
> Author: nwhitehorn
> Date: Sat Nov 25 23:41:05 2017
> New Revision: 326218
> URL: https://svnweb.freebsd.org/changeset/base/326218
>
> Log:
> Remove some, but not all, assumptions that the BSP is CPU 0 and that CPUs
> are numbered densely from there to n_cpus.
>
> MFC after: 1 month
>
> Modified:
> head/sys/kern/kern_clock.c
> head/sys/kern/kern_clocksource.c
> head/sys/kern/kern_shutdown.c
> head/sys/kern/kern_timeout.c
> head/sys/kern/sched_ule.c
> head/sys/kern/subr_pcpu.c
>
> Modified: head/sys/kern/kern_clock.c
> ==============================================================================
> --- head/sys/kern/kern_clock.c Sat Nov 25 23:23:24 2017
> (r326217)
> +++ head/sys/kern/kern_clock.c Sat Nov 25 23:41:05 2017
> (r326218)
> @@ -573,7 +573,9 @@ hardclock_cnt(int cnt, int usermode)
> void
> hardclock_sync(int cpu)
> {
> - int *t = DPCPU_ID_PTR(cpu, pcputicks);
> + int *t;
> + KASSERT(!CPU_ABSENT(cpu), ("Absent CPU %d", cpu));
Blank line before the KASSERT() perhaps?
> + t = DPCPU_ID_PTR(cpu, pcputicks);
>
> *t = ticks;
Probably don't need this blank line though?
> }
>
> Modified: head/sys/kern/sched_ule.c
> ==============================================================================
> --- head/sys/kern/sched_ule.c Sat Nov 25 23:23:24 2017 (r326217)
> +++ head/sys/kern/sched_ule.c Sat Nov 25 23:41:05 2017 (r326218)
> @@ -2444,6 +2451,7 @@ sched_add(struct thread *td, int flags)
> * Pick the destination cpu and if it isn't ours transfer to the
> * target cpu.
> */
> + td_get_sched(td)->ts_cpu = curcpu; /* Pick something valid to start */
> cpu = sched_pickcpu(td, flags);
It is not obvious why every sched_add() needs this once you've fixed thread0.
Shouldn't new threads just inherit from thread0's already-fixed value? If not,
perhaps fix thread0's value sooner?
> tdq = sched_setcpu(td, cpu, flags);
> tdq_add(tdq, td, flags);
>
> Modified: head/sys/kern/subr_pcpu.c
> ==============================================================================
> --- head/sys/kern/subr_pcpu.c Sat Nov 25 23:23:24 2017 (r326217)
> +++ head/sys/kern/subr_pcpu.c Sat Nov 25 23:41:05 2017 (r326218)
> @@ -279,6 +279,8 @@ pcpu_destroy(struct pcpu *pcpu)
> struct pcpu *
> pcpu_find(u_int cpuid)
> {
> + KASSERT(cpuid_to_pcpu[cpuid] != NULL,
> + ("Getting uninitialized PCPU %d", cpuid));
This KASSERT seems unnecessary? If the caller uses an invalid one, it will
just fault anyway.
> return (cpuid_to_pcpu[cpuid]);
> }
> @@ -409,7 +411,7 @@ DB_SHOW_ALL_COMMAND(pcpu, db_show_cpu_all)
> int id;
>
> db_printf("Current CPU: %d\n\n", PCPU_GET(cpuid));
> - for (id = 0; id <= mp_maxid; id++) {
> + CPU_FOREACH(id) {
If you remove the KASSERT you don't need this change since it checks the return
value of pcpu_find() (which you didn't change). In particular, this DDB command
shows all valid pcpu structures safely even if that set is inconsistent with
the all_cpus mask (or the old version did at least). There is also nothing
about
this that assumes BSP == 0 either. CPU_FOREACH() is doing a loop from 0 to
mp_maxid under the covers as well.
> pc = pcpu_find(id);
> if (pc != NULL) {
> show_pcpu(pc);
>
--
John Baldwin
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
