From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH] MCE: Fix race condition in mctelem_reserve Date: Wed, 22 Jan 2014 13:05:11 +0000 Message-ID: <52DFC207.3000805@citrix.com> References: <1390387834.32296.1.camel@hamster.uk.xensource.com> <52DFA3F1.4030303@citrix.com> <1390388414.32296.4.camel@hamster.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1390388414.32296.4.camel@hamster.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Frediano Ziglio Cc: Liu Jinsong , Christoph Egger , Jan Beulich , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 22/01/14 11:00, Frediano Ziglio wrote: > On Wed, 2014-01-22 at 10:56 +0000, David Vrabel wrote: >> On 22/01/14 10:50, Frediano Ziglio wrote: >>> These lines (in mctelem_reserve) >>> >>> >>> newhead = oldhead->mcte_next; >>> if (cmpxchgptr(freelp, oldhead, newhead) == oldhead) { >>> >>> are racy. After you read the newhead pointer it can happen that another >>> flow (thread or recursive invocation) change all the list but set head >>> with same value. So oldhead is the same as *freelp but you are setting >>> a new head that could point to whatever element (even already used). >>> >>> This patch use instead a bit array and atomic bit operations. >>> >>> Actually it use unsigned long instead of bitmap type as testing for >>> all zeroes is easier. >> >> bitmap_zero() does what you want. >> >> David > > No, bitmap_zero fills with zero, do not check for zeroes. bitmap_empty() sorry. David