Pablo Neira Ayuso wrote:
I'll do in a couple of minutes: I have another issue that you're going
to notice in the new patchset, I put it here before posting them if you
have time to look at it. I'd like to know if you're OK with this.
events = xchg(&e->cache, 0);
[...]
- notify->fcn(events, &item);
+ ret = notify->fcn(events, &item);
+ if (likely(ret == 0))
+ delivered = 1;
+ }
+ if (unlikely(!delivered)) {
+ unsigned int old = 0;
+retry:
+ /* restore undelivered events to the cache */
+ ret = cmpxchg(&e->cache, old, events);
+ /* ... but we've got new events during delivery */
+ if (unlikely(ret != old)) {
+ old = ret;
+ events |= ret;
+ goto retry;
+ }
}
out_unlock:
To avoid races between the cache clearing and event delivery:
1) I retrieve the event cache and clear it with xchg.
2) Then, if I fail to deliver the event, I restore the cache. However,
if the event cache is not zero anymore, then some new events have been
cached during the delivery, I use cmpxchg to conditionally restore the
events without losing the new events.
Can you see any problem with this optimistic approach? I know, it can
potentially try to restore the cache, but this is very unlikely to happen?
Its probably quite unlikely, so not a big issue. OTOH this is
effectively doing something quite similar to a spinlock. Maybe
its finally time to add per-conntrack locking.
Eric even had a patch for this IIRC :)
--
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
