Date: Wed, 26 Aug 2015 18:31:59 +0200 From: Julien Charbon <jch@freebsd.org> To: Hans Petter Selasky <hps@selasky.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, John Baldwin <jhb@freebsd.org> Subject: Re: svn commit: r286880 - head/sys/kern Message-ID: <55DDE9FF.3020705@freebsd.org> In-Reply-To: <55DD69E5.4090904@selasky.org> References: <201508181015.t7IAFAex055889@repo.freebsd.org> <55DD69E5.4090904@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --sVp8OSs2S8XEkQVM2w1JquIHOlmb6Xjm1 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Hi Hans, On 26/08/15 09:25, Hans Petter Selasky wrote: > 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, a= nd >> 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 >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> >> --- head/sys/kern/kern_timeout.c Tue Aug 18 10:07:03 2015 (r2868= 79) >> +++ head/sys/kern/kern_timeout.c Tue Aug 18 10:15:09 2015 (r2868= 80) >> @@ -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 progre= ss >> + * and indeed impossible to stop then return 0. >> + */ >> + not_running =3D !(cc_exec_curr(cc, direct) =3D=3D c); >> + >> CC_UNLOCK(cc); >> - return (1); >> + return (not_running); >> } >> >> void >=20 > 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: >=20 > not_running =3D !(cc_exec_curr(cc, direct) =3D=3D c && use_lock =3D= =3D 0); >=20 > Because if the callback process waits for lock "c_lock" in the callback= > process then we have above "cc_exec_curr(cc, direct) =3D=3D c" is satis= fied > too, and non-MPSAFE callouts are always cancelable, via > cc_exec_cancel(cc, direct) =3D true; Hum, interesting let me double check that. -- Julien --sVp8OSs2S8XEkQVM2w1JquIHOlmb6Xjm1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org iQEcBAEBCgAGBQJV3eoEAAoJEKVlQ5Je6dhxrnkH/3U43xxFECX8nd6l8PHlUhAF DwBvlKviCZ7fxnmUEEN8o1U/TC52E2hQYPVlONTmGkbEkOI7bppXOPeT8tcIJ38F u1uqwx1ArnG8T8Vne5puvB28Ij1SwyIarhkR4XUEAmYdp7IUrE5dIOhVeuVOTzSZ P3xoxrjMKfxaNtY+c7bDPuGbhzGFZiGK+ljsSltjxVKgPyXiKERyz3t51AYWJkMX npfn6iNLHkr3SePj03MzIL8e7aXtzCEiJSzENQDEdl+QlhuqYBv6TO1FGfMr/gP/ pAL2qoHpn84zuKsp9D+D4g7giW44Gd19RLx+DQxFLNSy3q9JMv1wr5cnc8Dq8ls= =cd2B -----END PGP SIGNATURE----- --sVp8OSs2S8XEkQVM2w1JquIHOlmb6Xjm1--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55DDE9FF.3020705>