Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756924AbbGTOok (ORCPT <rfc822;w@1wt.eu>);
	Mon, 20 Jul 2015 10:44:40 -0400
Received: from aserp1040.oracle.com ([141.146.126.69]:23924 "EHLO
	aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756502AbbGTOoh (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 20 Jul 2015 10:44:37 -0400
Date: Mon, 20 Jul 2015 16:44:05 +0200
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
        benh@kernel.crashing.org, davem@davemloft.net,
        jose.marchesi@oracle.com, sowmini.varadhan@oracle.com,
        dave.kleikamp@oracle.com
Subject: Re: [PATCH] iommu-common: Do not use 64 bit  constant
 0xffffffffffffffffl for computing align_mask
Message-ID: <20150720144405.GA2175@oracle.com>
References: <20150719121653.GA30645@oracle.com>
 <20150719152724.GB3729@roeck-us.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150719152724.GB3729@roeck-us.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-Source-IP: aserv0022.oracle.com [141.146.126.234]
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1289
Lines: 36

On (07/19/15 08:27), Guenter Roeck wrote:
> > -		align_mask = 0xffffffffffffffffl >> (64 - align_order);
> > +		align_mask = ~0l >> (64 - align_order);
> >  
> Wonder if this just hides the real problem. Unless align_order
> is very large, the resulting mask on 32 bit systems may be 0.
> Is this really the idea ?

<subsequent example code deleted> 

> So either case ~0l appears to be wrong; it should be ~0ul.
> I don't know if ~0ull makes a difference for some architectures. 

I agree about the unsigned part. However, regarding the arch specific
twists..

I checked into this.. even though I have a test program on 
x86_64 that "does the right thing" for both of 

  align_mask = ~0ul >> (64 - align_order);
  align_mask = ~0ul >> (BITS_PER_LONG - align_order);

when I compiled with -m32 and without (I tried align_order == 1 and 31
for edge cases), I think there are some gcc/arch specific variations
possible based on undefined behavior, so that the second variant 
is safer.

I'll send out a patch with that version soon.

--Sowmini

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/