On 12/24/06, James McIlree <jmcilree at apple.com>
wrote:>
> I''ve been reading still more code :-).
>
> I notice in fasttrap_disable_callbacks(), there is this snippet of
> code:
>
> for (cur = cpu->cpu_next_onln; cur != cpu;
> cur = cur->cpu_next_onln) {
> rw_enter(&cur->cpu_ft_lock, RW_WRITER);
> }
>
> dtrace_pid_probe_ptr = NULL;
> dtrace_return_probe_ptr = NULL;
>
> for (cur = cpu->cpu_next_onln; cur != cpu;
> cur = cur->cpu_next_onln) {
> rw_exit(&cur->cpu_ft_lock);
> }
>
> I think this is trying to make the caller wait for any outstanding
> fasttrap "traps" to finish, if they''ve already started.
>
> It looks to me like we lock every cpu except the one we are
currently
> executing on. Is this safe? Is there a guarantee we cannot be evicted or
> preempted from the current cpu between those two loops?
This does indeed look wrong; the call to rw_enter() could cause the
process to sleep, and there''s no guarantee it will wake up on the same
CPU.
It seems like the loops should be:
cur = cpu;
do {
rw_{enter,exit}(...);
} while ((cur = cur->cpu_next_onln) != cpu);
Seems like a bug should be filed.
Cheers,
- jonathan
