From owner-svn-src-all@freebsd.org Thu Aug 27 08:18:00 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 49EC49C2CC1 for ; Thu, 27 Aug 2015 08:18:00 +0000 (UTC) (envelope-from julien@jch.io) Received: from mail-qk0-f180.google.com (mail-qk0-f180.google.com [209.85.220.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 08B8F12B8 for ; Thu, 27 Aug 2015 08:17:59 +0000 (UTC) (envelope-from julien@jch.io) Received: by qkch123 with SMTP id h123so6371367qkc.0 for ; Thu, 27 Aug 2015 01:17:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-type; bh=JX3t/0O9JaDNI2XECs0rLkvBlCzznMHztpGIubM9QxU=; b=atjYVJ52Nm77IQS9UwOat1WLvoHuwoefQNpJBJRjf/hHRDcVuoRoU3cdHSryFqqNZJ hUX3UGxfqj21LcTsSD8wjM49CH37zaLF+29q0hcJLc0EiKTaS2HfJFQmKSBnZNecZ+eS jsfxFdwunZRLlI5U7GaC9qTj6MckcwCKQrWMHYxd8AY3dSBktHybfPIOvX4LaCRLmXzB Pl3i4yjxEOIaeom+ypD/4H7Lr8L7kEAFXdJeh3qbXWHVwpSMAUyDMACn+Cnr1h3Jd2hd kPos9K1xmEjP14m8NTesNQIuPOsW8uVNajAg0EsvatzyNHIs9ZsL/g1oFTNcCiwvE3qf k6Fg== X-Gm-Message-State: ALoCoQkXxXJ6O4YUk/Iuwtmpl78FcaMsLJ/31cjOm4Fs5hYTy0iigSOqsrV5uNnJwCXC8Vtc3vT4 X-Received: by 10.55.41.10 with SMTP id p10mr4523258qkh.28.1440663473638; Thu, 27 Aug 2015 01:17:53 -0700 (PDT) Received: from FRI2JCHARBON-M1.local (h87.s239.verisign.com. [216.168.239.87]) by smtp.googlemail.com with ESMTPSA id 124sm848720qhr.36.2015.08.27.01.17.50 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 27 Aug 2015 01:17:52 -0700 (PDT) Subject: Re: svn commit: r286880 - head/sys/kern To: Hans Petter Selasky , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, John Baldwin References: <201508181015.t7IAFAex055889@repo.freebsd.org> <55DD69E5.4090904@selasky.org> <55DDE9FF.3020705@freebsd.org> From: Julien Charbon X-Enigmail-Draft-Status: N1110 Message-ID: <55DEC7A2.1050808@freebsd.org> Date: Thu, 27 Aug 2015 10:17:38 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <55DDE9FF.3020705@freebsd.org> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="qGtiUEH0RGUTqXWLtKpNHUkFQHUk750Jr" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Aug 2015 08:18:00 -0000 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--