[RFC] callout overhaul: part I

Hello,
Some benchmarks posted by rwatson some time ago (
http://lists.freebsd.org/pipermail/freebsd-arch/2007-October/006945.html
) evicted callout_lock spinlock as an highly contented lock, in
particular for network paths. Honestly, this is not a surprise at all
:). This moved me in the direction of working on callouts, mainly for
improving the scalability respect a large number of CPUs but, since
callout are rather sensitive for the whole system, a general overhaul
to the whole mecanism would be a good idea.
In particular, I found phk's ideas about it (
http://lists.freebsd.org/pipermail/freebsd-arch/2006-November/005730.html
) very valuable and I have on-going discussions with him and rwatson
about these.

phk's proposal is however not complete at 100% and I'm trying to
integrate with other missing supports.
This patch, for example:
http://people.freebsd.org/~attilio/callout_rwlock.diff

would add the support for callout working with rwlock. Currently, mutex are the
only possible kind of lock to be used in conjuction with callouts and
this is a little bit limitative.

The support is implemented using the jhb's abstraction layer for
locks. Even if it is rather generic to be used with any kind of
abstracted lock, only rwlock and mutex are allowed to work because of
softclock() running in a kthread.
A new function is added to the KPI called callout_init_rw() which
basically does the same of callout_init_mtx() but with a rwlock.
Addictionally, it can get the new flag CALLOUT_SHAREDLOCK which let
acquiring the rwlock in shared mode while running the callout.
Using CALLOUT_SHAREDLOCK with callout_init_mtx() has no effect. We
could panic for it if someone it worths, but it would end up polluting
callout.h or alternatively complicating new _callout_init_lock().

Please, also note that the mtx_owned() stuff from the callout_stop()
function is removed as theoretically the lock, when specified, should
always be acquired when running callout_stop(). However, as there is
no simple way to do assertions with the lock abstraction class, and
there is no way to retrieve a child object from the lock_object class,
don't do any assertion for the moment, just comment the situation.
I'm eager to hear for your feedbacks.

Thanks,
Attilio

PS: It seems that other developers alredy did something similar (like
pjd. It seems we discussed about it some time ago but I completely
forgot about that... :)), please report if you have had better ideas
on it.


--
Peace can only be achieved by understanding - A. Einstein
_______________________________________________
freebsd-arch@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@xxxxxxxxxxx"

Relevant Pages

  • Re: em0 panic: mutex em0 not owned
    ... this is a race access to callout_stopin em driver. ... basically em_stopboth wants to stop core callout and tx channel ... callout but it only holds core lock. ...
    (freebsd-current)
  • Re: em0 panic: mutex em0 not owned
    ... basically em_stopboth wants to stop core callout and tx channel ... callout but it only holds core lock. ... patch now so the ball is in jfv@ court. ...
    (freebsd-current)
  • Re: em0 panic: mutex em0 not owned
    ... basically em_stopboth wants to stop core callout and tx channel ... callout but it only holds core lock. ... patch now so the ball is in jfv@ court. ...
    (freebsd-current)
  • Re: cvs commit: src/sys/kern kern_timeout.c src/sys/sys callout.h
    ... Add the function callout_init_rw() to callout facility in order to use ... rwlocks in conjuction with callouts. ... CALLOUT_SHAREDLOCK flag can be used, now, in order to acquire the lock ... currently the allowed lock classes are mutexes and rwlocks because ...
    (freebsd-current)
  • Status Update - Vol01, Issue 00 (12/18/2004)
    ... Covers previous week of commits ... o We are not required to initialize an invalid rwlock. ... all resources associated with the lock. ... Fix an upper-vnode leak created in revision 1.52. ...
    (freebsd-current)