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).
3) Start a timeout using this same callout that calls the function (async
drain function) that the user supplied *instead* of
the original callout.
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.
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.
I wonder how tested this code is?
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.
R
On Sep 14, 2015, at 3:52 AM, Hans Petter Selasky <hsela...@freebsd.org> wrote:
> Author: hselasky
> Date: Mon Sep 14 10:52:26 2015
> New Revision: 287780
> URL: https://svnweb.freebsd.org/changeset/base/287780
>
> Log:
> Implement callout_drain_async(), inspired by the projects/hps_head
> branch.
>
> This function is used to drain a callout via a callback instead of
> blocking the caller until the drain is complete. Refer to the
> callout_drain_async() manual page for a detailed description.
>
> Limitation: If a lock is used with the callout, the callout can only
> be drained asynchronously one time unless the callout_init_mtx()
> function is called again. This limitation is not present in
> projects/hps_head and will require more invasive changes to the
> timeout code, which was not in the scope of this patch.
>
> Differential Revision: https://reviews.freebsd.org/D3521
> Reviewed by: wblock
> MFC after: 1 month
>
> Modified:
> head/share/man/man9/Makefile
> head/share/man/man9/timeout.9
> head/sys/kern/kern_timeout.c
> head/sys/sys/_callout.h
> head/sys/sys/callout.h
>
> Modified: head/share/man/man9/Makefile
> ==============================================================================
> --- head/share/man/man9/Makefile Mon Sep 14 10:28:47 2015
> (r287779)
> +++ head/share/man/man9/Makefile Mon Sep 14 10:52:26 2015
> (r287780)
> @@ -1641,6 +1641,7 @@ MLINKS+=timeout.9 callout.9 \
> timeout.9 callout_active.9 \
> timeout.9 callout_deactivate.9 \
> timeout.9 callout_drain.9 \
> + timeout.9 callout_drain_async.9 \
> timeout.9 callout_handle_init.9 \
> timeout.9 callout_init.9 \
> timeout.9 callout_init_mtx.9 \
>
> Modified: head/share/man/man9/timeout.9
> ==============================================================================
> --- head/share/man/man9/timeout.9 Mon Sep 14 10:28:47 2015
> (r287779)
> +++ head/share/man/man9/timeout.9 Mon Sep 14 10:52:26 2015
> (r287780)
> @@ -36,6 +36,7 @@
> .Nm callout_active ,
> .Nm callout_deactivate ,
> .Nm callout_drain ,
> +.Nm callout_drain_async ,
> .Nm callout_handle_init ,
> .Nm callout_init ,
> .Nm callout_init_mtx ,
> @@ -70,6 +71,8 @@ typedef void timeout_t (void *);
> .Fn callout_deactivate "struct callout *c"
> .Ft int
> .Fn callout_drain "struct callout *c"
> +.Ft int
> +.Fn callout_drain_async "struct callout *c" "callout_func_t *fn" "void *arg"
> .Ft void
> .Fn callout_handle_init "struct callout_handle *handle"
> .Bd -literal
> @@ -264,6 +267,24 @@ fully stopped before
> .Fn callout_drain
> returns.
> .Pp
> +The function
> +.Fn callout_drain_async
> +is non-blocking and works the same as the
> +.Fn callout_stop
> +function.
> +When this function returns non-zero, do not call it again until the callback
> function given by
> +.Fa fn
> +has been called with argument
> +.Fa arg .
> +Only one of
> +.Fn callout_drain
> +or
> +.Fn callout_drain_async
> +should be called at a time to drain a callout.
> +If this function returns zero, it is safe to free the callout structure
> pointed to by the
> +.Fa c
> +argument immediately.
> +.Pp
> The
> .Fn callout_reset
> and
>
> Modified: head/sys/kern/kern_timeout.c
> ==============================================================================
> --- head/sys/kern/kern_timeout.c Mon Sep 14 10:28:47 2015
> (r287779)
> +++ head/sys/kern/kern_timeout.c Mon Sep 14 10:52:26 2015
> (r287780)
> @@ -1145,6 +1145,45 @@ callout_schedule(struct callout *c, int
> }
>
> int
> +callout_drain_async(struct callout *c, callout_func_t *func, void *arg)
> +{
> + struct callout_cpu *cc;
> + struct lock_class *class;
> + int retval;
> + int direct;
> +
> + /* stop callout */
> + callout_stop(c);
> +
> + /* check if callback is being called */
> + cc = callout_lock(c);
> + if (c->c_iflags & CALLOUT_DIRECT) {
> + direct = 1;
> + } else {
> + direct = 0;
> + }
> + retval = (cc_exec_curr(cc, direct) == c);
> +
> + /* drop locks, if any */
> + if (retval && c->c_lock != NULL &&
> + c->c_lock != &Giant.lock_object) {
> + /* ensure we are properly locked */
> + class = LOCK_CLASS(c->c_lock);
> + class->lc_assert(c->c_lock, LA_XLOCKED);
> + /* the final callback should not be called locked */
> + c->c_lock = NULL;
> + c->c_iflags |= CALLOUT_RETURNUNLOCKED;
> + }
> + CC_UNLOCK(cc);
> +
> + /* check if we should queue final callback */
> + if (retval)
> + callout_reset(c, 1, func, arg);
> +
> + return (retval);
> +}
> +
> +int
> _callout_stop_safe(struct callout *c, int safe)
> {
> struct callout_cpu *cc, *old_cc;
>
> Modified: head/sys/sys/_callout.h
> ==============================================================================
> --- head/sys/sys/_callout.h Mon Sep 14 10:28:47 2015 (r287779)
> +++ head/sys/sys/_callout.h Mon Sep 14 10:52:26 2015 (r287780)
> @@ -46,6 +46,8 @@ LIST_HEAD(callout_list, callout);
> SLIST_HEAD(callout_slist, callout);
> TAILQ_HEAD(callout_tailq, callout);
>
> +typedef void callout_func_t(void *);
> +
> struct callout {
> union {
> LIST_ENTRY(callout) le;
>
> Modified: head/sys/sys/callout.h
> ==============================================================================
> --- head/sys/sys/callout.h Mon Sep 14 10:28:47 2015 (r287779)
> +++ head/sys/sys/callout.h Mon Sep 14 10:52:26 2015 (r287780)
> @@ -82,6 +82,7 @@ struct callout_handle {
> #define callout_active(c) ((c)->c_flags & CALLOUT_ACTIVE)
> #define callout_deactivate(c) ((c)->c_flags &= ~CALLOUT_ACTIVE)
> #define callout_drain(c) _callout_stop_safe(c, 1)
> +int callout_drain_async(struct callout *, callout_func_t *, void *);
> void callout_init(struct callout *, int);
> void _callout_init_lock(struct callout *, struct lock_object *, int);
> #define callout_init_mtx(c, mtx, flags)
> \
>
--------
Randall Stewart
r...@netflix.com
803-317-4952
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
