Updated rusage patch

Bruce Evans brde at optusnet.com.au
Fri Jun 1 08:54:38 UTC 2007


On Fri, 1 Jun 2007, Bruce Evans wrote:

> Hmm, this is confusing.  Normal locking is not used for thread-local
> fields.  Instead, a side effect of spinlocking is used:
> mtx_lock_spin(&any) in non-interrupt code has the side effect of
> masking interrupts on the current CPU, so statclock() can access
> anything on the current CPU without acquiring any locks, just like
> an interrupt handler on a UP system.  This is used for td_*ticks.
> It doesn't work for ru_*rss since since exit1() doesn't hold a
> spinlock when copying td_ru.  The sched_locking of ru_*rss in
> statclock() doesn't help -- I think it now does nothing except
> waste time, since these fields are now thread-local so they need
> the same locking as td_*ticks, which is null in statclock() but
> non-null elsewhere.
>
> Related bugs:
> - td_[usip]ticks are still under (j) (sched_lock) in proc.h.
> - td_(uu,us}ticks have always (?) been under (k) (thread-local).  That
>  is more correct than (j), but they are updated by an interrupt handler
>  and seem to be accessed without disabling interrupts elsewhere.

Oops, it's more confusing than that.  It is not a bug for td_[usip]ticks
to be under sched_lock, since they must be under a more specific lock
than `any' for when they are reset in ruxagg().  That lock has the dual
purpose of locking out interrupts as a side effect and locking out other
threads explicitly.

Please add a lock assertion in ruxagg() and friends that the relevant
lock is held.

Bruce


More information about the freebsd-arch mailing list