From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] x86: fix variable_test_bit() asmconstraints Date: Fri, 14 Mar 2008 13:51:26 +0000 Message-ID: <47DA90EE.76E4.0078.0@novell.com> References: <47DA6E53.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org >>> Keir Fraser 14.03.08 12:55 >>> >On 14/3/08 11:23, "Jan Beulich" wrote: > >> I just sent a (much bigger, see below) patch to the same effect to the >> x86 Linux maintainers - in Xen, all the operations modifying bits do >> have "memory" clobbers, so it's just the test_bit() constraint that's >> wrong. > >Yes, I agree that fix is needed, although it would be consistent with = other >asm statements in that file to use a memory clobbers instead. But memory clobber aren't nice when you want the compiler to optimize well. >> However, I wonder whether the non-atomic ops aren't limiting things too >> much by having "memory" clobbers, they would much better be restricted >> to indicate just the changing memory location. This, however, would >> probably require some additional consideration given that Xen (other >> than Linux) isn't using -fno-strict-aliasing. Furthermore, these >> non-atomic operations, according to their comments, can be re-ordered, >> which contradicts the use of __asm__ __volatile__ (but note that >> removing this would probably require extra precautions to meet strict >> aliasing rules). > >Xen *does* use -fno-strict-aliasing. I'm afraid I don't understand why Oh, okay, I just grepped xen/ for it, but it lives in Config.mk... >memory-clobber would be needed for the atomic ops, but a dummy memory >operand would suffice for non-atomic ops. I used memory clobber everywhere= Atomic ops imply a barrier (otherwise the compiler can defeat the purpose of the atomic operation). The non-atomic ones don't need a dummy operand, but one that precisely describes the place in memory that changes. >because at least I'm certain that works. Same for 'asm volatile'. We've = had >various problems with the bitops before, and I just want the darn things = to >work! Okay, if you feel that way then I guess I won't propose changing it, at the expense of slightly worse code generation. Jan