From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54597)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1Yl2U1-0004fH-H7
	for qemu-devel@nongnu.org; Wed, 22 Apr 2015 17:47:02 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1Yl2Tw-0006IC-Hx
	for qemu-devel@nongnu.org; Wed, 22 Apr 2015 17:47:01 -0400
Received: from out4-smtp.messagingengine.com ([66.111.4.28]:34165)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1Yl2Tw-0006I2-EF
	for qemu-devel@nongnu.org; Wed, 22 Apr 2015 17:46:56 -0400
Received: from compute4.internal (compute4.nyi.internal [10.202.2.44])
	by mailout.nyi.internal (Postfix) with ESMTP id 89B3120748
	for <qemu-devel@nongnu.org>; Wed, 22 Apr 2015 17:46:55 -0400 (EDT)
Date: Wed, 22 Apr 2015 17:47:25 -0400
From: "Emilio G. Cota" <cota@braap.org>
Message-ID: <20150422214725.GA22985@flamenco>
References: <20150422164701.GA2378@flamenco>
	<1429721610-8251-1-git-send-email-cota@braap.org>
	<553804DF.10900@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <553804DF.10900@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2] translate-all: use bitmap helpers for
 PageDesc's bitmap
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org

On Wed, Apr 22, 2015 at 22:30:23 +0200, Paolo Bonzini wrote:
> On 22/04/2015 18:53, Emilio G. Cota wrote:
> > @@ -1221,8 +1194,9 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
> >          return;
> >      }
> >      if (p->code_bitmap) {
> > -        offset = start & ~TARGET_PAGE_MASK;
> > -        b = p->code_bitmap[offset >> 3] >> (offset & 7);
> > +        int nr = start & ~TARGET_PAGE_MASK;
> 
> This should be unsigned, otherwise the compiler might not make
> BIT_WORD(nr) a simple right shift (maybe it can see that nr > 0 thanks
> to the AND, but in principle x / 32 is not the same as x >> 5 if x is
> signed).

OK. Note that all bit{map,ops} helpers take a long as the bit position,
and then do BIT_WORD() on it--I was following that convention wrt sign.

> > +        int b = p->code_bitmap[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG - 1));
> 
> And this should be unsigned long.

Used an int because we only care about the least significant bits (len <= 8):

> >          if (b & ((1 << len) - 1)) {
> >              goto do_invalidate;
> >          }

But yes, an unsigned long here is more appropriate.

Thanks,

		Emilio