On 08/18/15 12:15, Julien Charbon wrote:
Author: jch
Date: Tue Aug 18 10:15:09 2015
New Revision: 286880
URL: https://svnweb.freebsd.org/changeset/base/286880
Log:
callout_stop() should return 0 (fail) when the callout is currently
being serviced and indeed unstoppable.
A scenario to reproduce this case is:
- the callout is being serviced and at same time,
- callout_reset() is called on this callout that sets
the CALLOUT_PENDING flag and at same time,
- callout_stop() is called on this callout and returns 1 (success)
even if the callout is indeed currently running and unstoppable.
This issue was caught up while making r284245 (D2763) workaround, and
was discussed at BSDCan 2015. Once applied the r284245 workaround
is not needed anymore and will be reverted.
Differential Revision: https://reviews.freebsd.org/D3078
Reviewed by: jhb
Sponsored by: Verisign, Inc.
Modified:
head/sys/kern/kern_timeout.c
Modified: head/sys/kern/kern_timeout.c
==============================================================================
--- head/sys/kern/kern_timeout.c Tue Aug 18 10:07:03 2015
(r286879)
+++ head/sys/kern/kern_timeout.c Tue Aug 18 10:15:09 2015
(r286880)
@@ -1150,7 +1150,7 @@ _callout_stop_safe(struct callout *c, in
struct callout_cpu *cc, *old_cc;
struct lock_class *class;
int direct, sq_locked, use_lock;
- int not_on_a_list;
+ int not_on_a_list, not_running;
if (safe)
WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, c->c_lock,
@@ -1378,8 +1378,15 @@ again:
}
}
callout_cc_del(c, cc);
+
+ /*
+ * If we are asked to stop a callout which is currently in progress
+ * and indeed impossible to stop then return 0.
+ */
+ not_running = !(cc_exec_curr(cc, direct) == c);
+
CC_UNLOCK(cc);
- return (1);
+ return (not_running);
}
void
Hi,
I think this patch is incomplete and can break the return value for
non-MPSAFE callouts. I think the correct statement should check the
value of "use_lock" too:
not_running = !(cc_exec_curr(cc, direct) == c && use_lock == 0);
Because if the callback process waits for lock "c_lock" in the callback
process then we have above "cc_exec_curr(cc, direct) == c" is satisfied
too, and non-MPSAFE callouts are always cancelable, via
cc_exec_cancel(cc, direct) = true;
class->lc_lock(c_lock, lock_status);
/*
* The callout may have been cancelled
* while we switched locks.
*/
if (cc_exec_cancel(cc, direct)) {
class->lc_unlock(c_lock);
goto skip;
}
/* The callout cannot be stopped now. */
cc_exec_cancel(cc, direct) = true;
--HPS
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"