[Differential] [Commented On] D1711: Changes to the callout code to restore active semantics and also add a test-framework and test to validate thecallout code (and potentially for use by other tests).

[rrs (Randall Stewart)]

rrs added a comment.

Julian:

The simple fix is to change the callout_init_rw -> callout_init(c, 1);

That makes it so the callout_stop() in this instance would return 0, not 1. Since the
callout could not be stopped.

This would then cause the reference count to drop to 1 (it was 2 the original and the callout).
It would not free here but when the callout runs it would.

Now that all being said, I have put that in some netflix code and will test it.. but there is
something strange about this whole lle* path that I don't yet grok.

The arp code and nd6 code carefully do reference counting. There are comments in
in.c and in6.c that mention that the arp/nd6 code are the callers.. thats fine.. However
*if* either callout goes off in the then the in/in6 callout runs which does:

UNLOCK(lle)
free(la)

No reference counting, no nothing.. I don't grok how all this is connected and
need to study it more but it just feels wrong. If its doing reference counting I would
have thought the callout would have done so too.

I need to take a bigger and closer view at the lle/arp code and how it is all
supposed to inter-work.. it appears to me broken, but that could be just my
mis-understanding of the code.. I should get home tomorrow and besides fixing
my mail-server I wills see what Robert, George and Kirk have to say about
the lle code paths in there new book, and then dig in and try to puzzle it out.

I *think* the simple change callout_init_rw -> callout_init(c, 1) will fix this and of
course we have to remove the unlock in the actual callout since the callout
code won't be locking it any longer... but I am wondering if there is not a 
deeper issue with this code. Doing the above will not change the behavior and
will get the right results on the flush, but this use/not-use references seems wrong
so there may be deeper bugs in the lle code!

R

REVISION DETAIL
  https://reviews.freebsd.org/D1711

To: rrs, gnn, rwatson, lstewart, jhb, kostikbel, sbruno, imp, adrian, hselasky
Cc: julian, hiren, jhb, kostikbel, emaste, delphij, neel, erj, freebsd-net