From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH] bitops: add _local bitops Date: Wed, 9 May 2012 19:55:16 +0300 Message-ID: <20120509165515.GA21461@redhat.com> References: <20120509134528.GA18044@redhat.com> <4FAA7939.6040706@zytor.com> <20120509154734.GB20867@redhat.com> <4FAA9A49.8080900@zytor.com> <20120509163641.GA21344@redhat.com> <4FAA9F45.1080608@zytor.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <4FAA9F45.1080608@zytor.com> Sender: linux-doc-owner@vger.kernel.org To: "H. Peter Anvin" Cc: Rob Landley , Thomas Gleixner , Ingo Molnar , x86@kernel.org, Arnd Bergmann , Andrew Morton , David Howells , Akinobu Mita , Alexey Dobriyan , Herbert Xu , Stephen Rothwell , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, Gleb Natapov , Paolo Bonzini , kvm@vger.kernel.org, Avi Kivity , Marcelo Tosatti , Linus Torvalds List-Id: linux-arch.vger.kernel.org On Wed, May 09, 2012 at 09:45:57AM -0700, H. Peter Anvin wrote: > On 05/09/2012 09:36 AM, Michael S. Tsirkin wrote: > > > > Well it talks about a memory barrier, not an > > optimization barrier. > > > > Same thing. I see. So it really should say 'any barrier', right? Documentation/atomic_ops.txt goes to great length to distinguish between the two and we probably should not confuse things. > > If compiler reorders code, changes will appear in > > the wrong order on the current processor, > > not just on other processors, no? > > Yes. So this seems to contradict what the comment says: clear_bit() is atomic and may not be reordered. and you say compiler *can* reorder it, and below you should call smp_mb__before_clear_bit() and/or * smp_mb__after_clear_bit() in order to ensure changes are visible on other processors. and in fact this is not enough, you also need to call barrier() to ensure changes are visible on the same processor in the correct order. > For your _local I would just copy the atomic bitops but remote the locks > in most cases. > > -hpa Right, I sent v2 that does exactly this. My question about documentation for change_bit is an unrelated one: to me, it looks like the documentation for change_bit does not match the implementation, or at least is somewhat confusing. > -- > H. Peter Anvin, Intel Open Source Technology Center > I work for Intel. I don't speak on their behalf. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:40830 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760304Ab2EIQzh (ORCPT ); Wed, 9 May 2012 12:55:37 -0400 Date: Wed, 9 May 2012 19:55:16 +0300 From: "Michael S. Tsirkin" Subject: Re: [PATCH] bitops: add _local bitops Message-ID: <20120509165515.GA21461@redhat.com> References: <20120509134528.GA18044@redhat.com> <4FAA7939.6040706@zytor.com> <20120509154734.GB20867@redhat.com> <4FAA9A49.8080900@zytor.com> <20120509163641.GA21344@redhat.com> <4FAA9F45.1080608@zytor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FAA9F45.1080608@zytor.com> Sender: linux-arch-owner@vger.kernel.org List-ID: To: "H. Peter Anvin" Cc: Rob Landley , Thomas Gleixner , Ingo Molnar , x86@kernel.org, Arnd Bergmann , Andrew Morton , David Howells , Akinobu Mita , Alexey Dobriyan , Herbert Xu , Stephen Rothwell , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, Gleb Natapov , Paolo Bonzini , kvm@vger.kernel.org, Avi Kivity , Marcelo Tosatti , Linus Torvalds Message-ID: <20120509165516.KVFuDs8ThQPXQ4gjGqfgQYWlSa_RyMh9y0XqOyDkO50@z> On Wed, May 09, 2012 at 09:45:57AM -0700, H. Peter Anvin wrote: > On 05/09/2012 09:36 AM, Michael S. Tsirkin wrote: > > > > Well it talks about a memory barrier, not an > > optimization barrier. > > > > Same thing. I see. So it really should say 'any barrier', right? Documentation/atomic_ops.txt goes to great length to distinguish between the two and we probably should not confuse things. > > If compiler reorders code, changes will appear in > > the wrong order on the current processor, > > not just on other processors, no? > > Yes. So this seems to contradict what the comment says: clear_bit() is atomic and may not be reordered. and you say compiler *can* reorder it, and below you should call smp_mb__before_clear_bit() and/or * smp_mb__after_clear_bit() in order to ensure changes are visible on other processors. and in fact this is not enough, you also need to call barrier() to ensure changes are visible on the same processor in the correct order. > For your _local I would just copy the atomic bitops but remote the locks > in most cases. > > -hpa Right, I sent v2 that does exactly this. My question about documentation for change_bit is an unrelated one: to me, it looks like the documentation for change_bit does not match the implementation, or at least is somewhat confusing. > -- > H. Peter Anvin, Intel Open Source Technology Center > I work for Intel. I don't speak on their behalf.