Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751602AbaKKE3R (ORCPT ); Mon, 10 Nov 2014 23:29:17 -0500 Received: from mail-yh0-f42.google.com ([209.85.213.42]:35362 "EHLO mail-yh0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751296AbaKKE3Q (ORCPT ); Mon, 10 Nov 2014 23:29:16 -0500 MIME-Version: 1.0 In-Reply-To: References: <1415644096-3513-1-git-send-email-j.glisse@gmail.com> <1415644096-3513-4-git-send-email-j.glisse@gmail.com> <20141110205814.GA4186@gmail.com> <20141110225036.GB4186@gmail.com> <20141111024531.GA2503@gmail.com> Date: Mon, 10 Nov 2014 20:29:15 -0800 X-Google-Sender-Auth: W4j--_tKwcjxixd70N6uNNtrgeI Message-ID: Subject: Re: [PATCH 3/5] lib: lockless generic and arch independent page table (gpt) v2. From: Linus Torvalds To: Jerome Glisse Cc: Andrew Morton , Linux Kernel Mailing List , linux-mm , Joerg Roedel , Mel Gorman , "H. Peter Anvin" , Peter Zijlstra , Andrea Arcangeli , Johannes Weiner , Larry Woodman , Rik van Riel , Dave Airlie , Brendan Conoboy , Joe Donohue , Duncan Poole , Sherry Cheung , Subhash Gutti , John Hubbard , Mark Hairgrove , Lucien Dunning , Cameron Buschardt , Arvind Gopalakrishnan , Shachar Raindel , Liran Liss , Roland Dreier , Ben Sander , Greg Stoner , John Bridgman , Michael Mantor , Paul Blinzer , Laurent Morichetti , Alexander Deucher , Oded Gabbay , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 10, 2014 at 7:16 PM, Linus Torvalds wrote: > > There's no reason for a "u64 cast". The value of "1 << pd_shift" is > going to be an "int" regardless of what type pd_shift is. The type of > a shift expression is the type of the left-hand side (with the C > promotion rules forcing it to at least "int"), the right-hand > expression type has absolutely no relevance. Btw, for that exact reason, code like this: + (uint64_t)(pdp->index + + (1UL << (gpt_pdp_shift(gpt, pdp) + gpt->pd_shift)) - 1UL)); is likely buggy if you actually care about the uint64_t part. On 32-bit, 1ul will be 32-bit. And so will "(1ul << .. ) -1UL", regardless of the type of the right hand of the shift. So the fact that gpt->pd_shift and gpt_pdp_shift() are both u64, the actual end result is u32 (page->index is a 32-bit entity on 32-bit architectures, since pgoff_t is an "unsigned long" too). So you're doing the shifts in 32-bit, the addition in 32-bit, and then just casting the resulting 32-bit thing to a 64-bit entity. The high 32 bits are guaranteed to be zero, in other words. This just highlights how wrong it is to make those shifts be u64. That gpt_pdp_shift() helper similarly should at no point be returning u64. It doesn't help, it only hurts. It makes the structure bigger for no gain, and apparently it confuses people into thinking those shifts are done in 64 bit. When you do "a+b" or similar operations, the end result is the biggest type size of 'a' and 'b' respectively (with the normal promotion to at least 'int'). But that's not true of shifts, the type of the shift expression is the (integer-promoted) left-hand side. The right-hand side just gives the amount that value is shifted by, it doesn't affect the type of the result. Linus -- 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/