From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754979AbYKKBTh (ORCPT ); Mon, 10 Nov 2008 20:19:37 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752817AbYKKBT3 (ORCPT ); Mon, 10 Nov 2008 20:19:29 -0500 Received: from mga09.intel.com ([134.134.136.24]:54534 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751488AbYKKBT2 (ORCPT ); Mon, 10 Nov 2008 20:19:28 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.33,580,1220252400"; d="asc'?scan'208";a="358497441" Subject: Re: [PATH -mm -v2] Fix a race condtion of oops_in_progress From: Huang Ying To: Chris Snook Cc: KOSAKI Motohiro , Andrew Morton , "linux-kernel@vger.kernel.org" In-Reply-To: <4918DB8E.2020004@redhat.com> References: <490F468B.4040602@redhat.com> <1225762877.27266.22.camel@yhuang-dev.sh.intel.com> <20081110163135.616E.KOSAKI.MOTOHIRO@jp.fujitsu.com> <1226365518.6081.90.camel@yhuang-dev.sh.intel.com> <4918DB8E.2020004@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-x3e4UxRLJxDORDFfAsEk" Date: Tue, 11 Nov 2008 09:19:18 +0800 Message-Id: <1226366358.6081.96.camel@yhuang-dev.sh.intel.com> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-x3e4UxRLJxDORDFfAsEk Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Tue, 2008-11-11 at 09:10 +0800, Chris Snook wrote: > Huang Ying wrote: > > On Mon, 2008-11-10 at 15:35 +0800, KOSAKI Motohiro wrote: > >>>>> As far as I know, barriers don't cause changes to be visible on oth= er > >>>>> CPUs faster too. It just guarantees corresponding operations after = will > >>>>> not get executed until that before have finished. And, I don't thin= k we > >>>>> need make changes to be visible on other CPUs faster. > >>>> You're correct that barrier() has no impact on other CPUs. wmb() an= d rmb() do.=20 > >>>> If we don't need to make changes visible any faster, what's the po= int in using=20 > >>>> atomic_set()? It's not any less racy. atomic_inc() and atomic_dec(= ) would be=20 > >>>> less racy, but you're not using those. > >>> In default bust_spinlocks() implementation in lib/bust_spinlocks.c, > >>> atomic_inc() and atomic_dec_and_test() is used. Which is used by x86 > >>> too. In some other architecture, atomic_set() is used to replace > >>> "oops_in_progress =3D ". So this patch fixes architectures which= use > >>> default bust_spinlocks(), other architectures can be fixed by > >>> corresponding architecture developers. > >> I think Chris is right. > >> So, I reccomend to read Documentation/memory-barriers.txt > >> > >> Almost architecture gurantee atomic_inc cause barrier implicitly. > >> but not _all_ architecture. > >=20 > > Yes. atomic_inc() doesn't imply barrier on all architecture. But we > > should not add barriers before all atomic_inc(), just ones needed. Can > > you figure out which ones in the patch should has barrier added? >=20 > You need barriers *after* writes, and *before* reads. Adding barriers to= the=20 > oops path should be extremely cheap for performance, unless oopsing is a = common=20 > occurrence, in which case we have bigger problems. I just suspect why we need these barriers. Do we have some memory must to be written after oops_in_progress? Or some memory must to be read before oops_in_progress? Best Regards, Huang Ying --=-x3e4UxRLJxDORDFfAsEk Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkkY3ZEACgkQKhFGF+eHlpipfwCcDALIdyyZ4a8sav7qvLuMdhSY jO0AoIcfRjCz0OvLOpW/qZexPvordsyT =5ahY -----END PGP SIGNATURE----- --=-x3e4UxRLJxDORDFfAsEk--