Date: Thu, 27 Aug 2015 10:17:38 +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: <55DEC7A2.1050808@freebsd.org> In-Reply-To: <55DDE9FF.3020705@freebsd.org> References: <201508181015.t7IAFAex055889@repo.freebsd.org> <55DD69E5.4090904@selasky.org> <55DDE9FF.3020705@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --qGtiUEH0RGUTqXWLtKpNHUkFQHUk750Jr Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Hi Hans, On 26/08/15 18:31, Julien Charbon wrote: > 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 currentl= y >>> 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 >>> =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 (r286= 879) >>> +++ head/sys/kern/kern_timeout.c Tue Aug 18 10:15:09 2015 (r286= 880) >>> @@ -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 progr= ess >>> + * 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 >> >> 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 =3D !(cc_exec_curr(cc, direct) =3D=3D c && use_lock =3D= =3D 0); >> >> Because if the callback process waits for lock "c_lock" in the callbac= k >> process then we have above "cc_exec_curr(cc, direct) =3D=3D c" is sati= sfied >> too, and non-MPSAFE callouts are always cancelable, via >> cc_exec_cancel(cc, direct) =3D true; >=20 > Hum, interesting let me double check that. After checking the non-mpsafe callout cases, you are completely right, this test makes sense only in mpsafe callout case. Fixed in r287196: https://svnweb.freebsd.org/base?view=3Drevision&revision=3D287196 Thanks a lot for your time. -- Julien --qGtiUEH0RGUTqXWLtKpNHUkFQHUk750Jr 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 iQEcBAEBCgAGBQJV3seuAAoJEKVlQ5Je6dhxDpwH/37Z4WzUVDiFmRhy3VWMUFze A4NPC2+wFWZZXsO5djBQtOyOD10aiZ7vNbkLWdy2aZ0/HbqIrMiDSYVPjQDoilU2 1Fkwy04PuvPGRMCG8bAUjW/MMXrAtKNAe2MsTWraVTcR0b1gHQ103txSyMPiVvL2 8zLa+2prHY2xkjFLjG4HYGo7y6phjzh8hSlsBKuwZmgLhEJn60CWq1b6iba2eL0I NZnTwfVbi7wyFUvRbvmWYbKrA2QyMt1heMV3YRyf2OkqGw6tVE9KQQzHHBYKht+q vr3HSx7ZDdXWs7Hn+EwNjisrhgZcPwwOcoXzaxuOQUu5fTnlflmC7GMmjMH83uc= =jp9c -----END PGP SIGNATURE----- --qGtiUEH0RGUTqXWLtKpNHUkFQHUk750Jr--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55DEC7A2.1050808>