cvs commit: src/sys/kern sched_ule.c

[Jeff Roberson]

On Wed, 24 Jan 2007, David Xu wrote:

> Jeff Roberson wrote:
>
>>> if idlethread is preempted, who will clear its idle bit in idle_cpus_mask 
>>> ?
>> 
>> 
>> idle_cpus_mask was broken before for all schedulers.  This commit didn't 
>> change that.  ULE doesn't use idle_cpus_mask and it's idlethread doesn't 
>> set or clear it.  The idle thread for the other schedulers remains 
>> unchanged.
>> 
> I havn't read ULE code, but 4BSD has following code:
>

This is the very same code that was there before.  I didn't change it.  I 
just moved it into the schedulers.  It was always broken with preemption. 
I tried to use idle_cpus_mask in ULE but found that it always was set for 
most CPUs.  You will find that IPI_PREEMPT also had no effect but I fixed 
that recently.

Jeff

> void
> sched_idletd(void *dummy)
> {
>        struct proc *p;
>        struct thread *td;
> #ifdef SMP
>        cpumask_t mycpu;
> #endif
>
>        td = curthread;
>        p = td->td_proc;
> #ifdef SMP
>        mycpu = PCPU_GET(cpumask);
>        mtx_lock_spin(&sched_lock);
>        idle_cpus_mask |= mycpu;
>        mtx_unlock_spin(&sched_lock);
> #endif
>        for (;;) {
>                mtx_assert(&Giant, MA_NOTOWNED);
>
>
> how about if the idlethread is preempted here ? mycpu is not cleared.
>
>
>                while (sched_runnable() == 0)
>                        cpu_idle();
>
>                mtx_lock_spin(&sched_lock);
> #ifdef SMP
>                idle_cpus_mask &= ~mycpu;
> #endif
>                mi_switch(SW_VOL, NULL);
> #ifdef SMP
>                idle_cpus_mask |= mycpu;
> #endif
>                mtx_unlock_spin(&sched_lock);
>        }
> }
>
>> Julian wants to set idle_cpus_mask in sched_switch() which would make it 
>> accurate no matter how idlethread switched out.  that seems much more 
>> reasonable to me.
>> 
>
> I haven't seen sched_switch clears it, so at least, it should be fixed for 
> current 4BSD scheduler.
>
>> Cheers,
>> Jeff
>
> Regards,
> David Xu
>