svn commit: r287780 - in head: share/man/man9 sys/kern sys/sys

[Hans Petter Selasky]

On 09/16/15 22:54, Randall Stewart wrote:
> Hans:
>
> By outside prompting, I have finally had a chance to look at this:
>
> What it appears you do here is:
>
> a) stop the callout, not paying attention to if it stopped or not.
> b) Then get the callout systems lock and check if your callout is up and running, storing that in your
>       retval. Where 1 is it was running, and 0 is no it was not running.
>   c) Assuming that your callout is running.
>     1) assert the user has the lock (if its a mtx associated lock), which means the callout is blocked on you finishing this.
>     2) change the nature of the callout so that it will return-unlocked (no matter what the caller thought he set in his callout).

Hi Randall,

2) I didn't want to touch the callout code, so this is the only way to 
do it without changing anything. You can complete the 
callout_async_drain() function which I believe you have promised to do?

>     3) Start a timeout using this same callout that calls the function (async drain function) that the user supplied *instead* of
>         the original callout.

3) Starting a callout after callout_async_drain() is not supported 
currently, except for MP-SAFE callouts (c_lock == NULL).

> Now this is *not* how I would have done it and I question c.2 especially. I don’t think
> you should be changing the nature of the callout and its lock.

Right, but that requires changes inside the callout_process() to handle 
a lock-less call to the destructor and I don't want to touch those parts.

> Overall I really doubt this will work well since the callout that you start will call to the user
> function that says “I am done with the callout memory.. which is usually what you want this async-drain for” but
> using the very memory that you want to free for the callout.

This is exactly what's being done in the TCP stack currently. No 
complaints about that!

The callout is not touched after the callback completion. All variables 
are copied to the stack beforehand. That's why we cannot have c_lock 
unequal to NULL, and must clear it and set CALLOUT returnulocked, else 
the a lock which might be inside freed memory will be unlocked.

> I wonder how tested this code is?

I've run the code through various tests over here, and it works like 
expected.

> I would think one would be better off having a way to set a callback if you are trying to stop and drain-async
> that the actual callout code itself calls to say “I am done”.. not this interesting recursive use
> of the callout itself.

Right. That part I've reserved for you, hence it involves touching the 
callout code! What we've got now is simply a replacement (factor out) 
for some code which is currently inside the TCP stack.


Thank you for your time reviewing this Randall.

--HPS