a proposed callout API

[Hans Petter Selasky]

On Monday 13 November 2006 21:53, Poul-Henning Kamp wrote:
> A number of problems have been identified with our current callout
> code and I have been thinking about and discussed various aspects
> with people during the EuroBSDcon2007 conference.
>
> A lot of people are interested in this, so here is a quick sketch
> of what I'm thinking about:
>
>
> The Problems
> ------------
>
> 1. We need better resolution than a periodic "hz" clock can give us.
>    Highspeed networking, gaming servers and other real-time apps want
>    this.
>
> 2. We "pollute" our call-wheel with tons of callouts that we know are
>    unlikely to happen.
>
> 3. We have many operations on the callout wheel because certain
>    callouts gets rearmed for later in the future.  (TCP keepalives).
>
> 4. We execute all callouts on one CPU only.
>
> 5. Most of the specified timeouts are bogus, because of the imprecision
>    inheret in the current 1/hz method of scheduling them.
>
> and a number of other issues.
>
>
> The proposed API
> ----------------
>
> tick_t XXX_ns_tick(unsigned nsec, unsigned *low, unsigned *high);
>  Caculate the tick value for a given timeout.
>  Optionally return theoretical lower and upper limits to
>  actual value,
>
> tick_t XXX_s_tick(unsigned seconds)
>  Caculate the tick value for a given timeout.
>
> The point behind these two functions is that we do not want to
> incur a scaling operating at every arming of a callout.  Very
> few callouts use varying timeouts (and for those, no avoidance
> is possible), but for the rest, precalculating the correct
> (opaque) number is a good optimization.
>

Hi,

I have some comments on the matter.

Is XXX_arm only an init routine? If not, it does not make sense that one is 
allowed to pass a variable mutex argument. This will lead to some locking 
problems I think. Also I am missing the CALLOUT_RETURNUNLOCKED flag.

> XXX_arm(struct xxx*, tick_t, func *, arg *, int flag, struct mtx *);
>  Arm timer.
>  Struct xxx must be zeroed before first call.
>
>  If mtx pointer is non-NULL, acq mutex before calling.

This makes sense. Should also require this for rearm/disarm, but not drain.

>
>  flags:
>   XXX_REPEAT
>   XXX_UNLIKELY
>
> Arm a callout with a number of optional behaviours specified.
>
> XXX_rearm(struct xxx*, tick_t)
>  Rearm timer.
>
> XXX_disarm(struct xxx*)
>  Unarm the timer.
>
> XXX_drain(struct xxx*)
>  Drain the timer.
>
>
> The functions above will actually be wrappers for a more generic
> set of the same family, which also takes a pointer to a callout-group.
>
> This is so that we can have different groups of callouts, for
> instance one group for the/each netstack and one for the disk-I/O
> stuff etc.

Why can't you just rename and extend the existing 
callout_init_mtx()/callout_reset()/callout_stop()/callout_drain() functions ?

Yours
--HPS