Ingo Molnar wrote:
* Patrick McHardy <kaber@xxxxxxxxx> wrote:
We need to avoid having a timer that was deleted by one CPU
getting re-added by another, but want to avoid taking the
conntrack lock for every timer update. The timer-internal
locking is enough for this as long as we have a mod_timer
variant that forwards a timer, but doesn't activate it in
case it isn't active already.
that makes sense - but the implementation is still somewhat
ugly. How about the one below instead? Not tested.
This seems to fulfill our needs. I also like the mod_timer_pending()
name better than mod_timer_noact().
One open question is this construct in mod_timer():
+ /*
+ * This is a common optimization triggered by the
+ * networking code - if the timer is re-modified
+ * to be the same thing then just return:
+ */
+ if (timer->expires == expires && timer_pending(timer))
+ return 1;
We've had this for ages, but it seems rather SMP-unsafe.
timer_pending(), if used in an unserialized fashion, can be any
random value in theory - there's no internal serialization here
anywhere.
We could end up with incorrectly not re-activating a timer in
mod_timer() for example - have such things never been observed
in practice?
Yes, it seems racy if done for timers that might get activated.
For forwarding only without activation it seems OK, in that case
the timer_pending check doesn't seem necessary at all.
So the original patch which added this to mod_timer_noact() was
racy i think, and we cannot preserve this optimization outside
of the timer list lock. (we could do it inside of it.)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
