Re: Ubiquitous ucas(9)

[Jason Thorpe <thorpej%me.com@localhost>]

> On Mar 31, 2019, at 3:12 PM, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost> wrote:
> 
> A few comments:
> 
> - Kernel preemption is implicitly disabled at IPL_HIGH, so you do not need
> to do it explicitly.

Ah, of course.  Makes sense.

> - Consider re-using the cpu_lock instead of adding ucas_critical_mutex.

Done.

> - Also, consider using kcpuset_copy(&ucas_kcpuset, kcpuset_attached),
> plus clearing the current CPU; ucas_kcpuset can be global and it would
> be already protected by cpu_lock.  Alternatively, you can add a third
> parameter to ipi_trigger_multi(), e.g. "bool exclude_curcpu", as it is
> arguably a common case.  Then you would just use kcpuset_attached.

I added a ipi_trigger_broadcast() that takes a bool "skip_self" argument.  (I added the same to ipi_broadcast() for symmetry.)

I also found what I think is a bug in ipi_trigger_multi() -- in the "send to self" case, it was calling the ipi_cpu_handler(), but never setting the target IPI as pending, at least that I could see.  I fixed this, even though I wanted to skip_self :-)

> - You can get away with one counter (eliminating ucas_critical_owning_cpu)
> if you set it to ncpu (well, kcpuset_countset(kcpuset_attached) - 1) and
> decrement.  Note: once it reaches zero, you can use -1 in order to release
> the waiters on other CPU.
> 
> - The counter must be volatile or accessed through a volatile cast.

Cool, I will tidy those up.

Thanks for the feedback.

-- thorpej