From: Linus Torvalds <torvalds@linux-foundation.org>
To: Jerome Glisse <j.glisse@gmail.com>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
linux-mm <linux-mm@kvack.org>, "Joerg Roedel" <joro@8bytes.org>,
"Mel Gorman" <mgorman@suse.de>, "H. Peter Anvin" <hpa@zytor.com>,
"Peter Zijlstra" <peterz@infradead.org>,
"Andrea Arcangeli" <aarcange@redhat.com>,
"Johannes Weiner" <jweiner@redhat.com>,
"Larry Woodman" <lwoodman@redhat.com>,
"Rik van Riel" <riel@redhat.com>,
"Dave Airlie" <airlied@redhat.com>,
"Brendan Conoboy" <blc@redhat.com>,
"Joe Donohue" <jdonohue@redhat.com>,
"Duncan Poole" <dpoole@nvidia.com>,
"Sherry Cheung" <SCheung@nvidia.com>,
"Subhash Gutti" <sgutti@nvidia.com>,
"John Hubbard" <jhubbard@nvidia.com>,
"Mark Hairgrove" <mhairgrove@nvidia.com>,
"Lucien Dunning" <ldunning@nvidia.com>,
"Cameron Buschardt" <cabuschardt@nvidia.com>,
"Arvind Gopalakrishnan" <arvindg@nvidia.com>,
"Shachar Raindel" <raindel@mellanox.com>,
"Liran Liss" <liranl@mellanox.com>,
"Roland Dreier" <roland@purestorage.com>,
"Ben Sander" <ben.sander@amd.com>,
"Greg Stoner" <Greg.Stoner@amd.com>,
"John Bridgman" <John.Bridgman@amd.com>,
"Michael Mantor" <Michael.Mantor@amd.com>,
"Paul Blinzer" <Paul.Blinzer@amd.com>,
"Laurent Morichetti" <Laurent.Morichetti@amd.com>,
"Alexander Deucher" <Alexander.Deucher@amd.com>,
"Oded Gabbay" <Oded.Gabbay@amd.com>,
"Jérôme Glisse" <jglisse@redhat.com>
Subject: Re: [PATCH 3/5] lib: lockless generic and arch independent page table (gpt) v2.
Date: Mon, 10 Nov 2014 20:29:15 -0800 [thread overview]
Message-ID: <CA+55aFwxdT+G26O3mSmrMuq5f3Tf9cRVhOLxR7kpXZu49x8c5Q@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFw5MdJVK5AWV39rorMsmuny00=jVaBrnMRAoKAxBeZO7Q@mail.gmail.com>
On Mon, Nov 10, 2014 at 7:16 PM, Linus Torvalds
<torvalds@linux-foundation.org> 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2014-11-11 4:29 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-10 18:28 HMM (heterogeneous memory management) v6 j.glisse
2014-11-10 18:28 ` [PATCH 1/5] mmu_notifier: add event information to address invalidation v6 j.glisse
2014-11-10 18:28 ` [PATCH 2/5] mmu_notifier: keep track of active invalidation ranges v2 j.glisse
2014-11-10 18:28 ` [PATCH 3/5] lib: lockless generic and arch independent page table (gpt) v2 j.glisse
2014-11-10 20:22 ` Linus Torvalds
2014-11-10 20:58 ` Jerome Glisse
2014-11-10 21:35 ` Linus Torvalds
2014-11-10 21:47 ` Linus Torvalds
2014-11-10 22:58 ` Jerome Glisse
2014-11-10 22:50 ` Jerome Glisse
2014-11-10 23:53 ` Linus Torvalds
2014-11-11 2:45 ` Jerome Glisse
2014-11-11 3:16 ` Linus Torvalds
2014-11-11 4:19 ` Jerome Glisse
2014-11-11 4:29 ` Linus Torvalds [this message]
2014-11-11 9:59 ` Peter Zijlstra
2014-11-11 13:42 ` Jerome Glisse
2014-11-11 21:01 ` David Airlie
2014-11-13 23:50 ` Linus Torvalds
2014-11-14 0:58 ` Kirill A. Shutemov
2014-11-14 1:18 ` Linus Torvalds
2014-11-14 1:50 ` Linus Torvalds
2014-11-13 16:07 ` Rik van Riel
2014-11-10 18:28 ` [PATCH 4/5] hmm: heterogeneous memory management v6 j.glisse
2014-11-11 19:00 ` HMM (heterogeneous memory management) v6 Christoph Lameter
2014-11-12 20:09 ` Jerome Glisse
2014-11-12 23:08 ` Christoph Lameter
2014-11-13 4:28 ` Jerome Glisse
-- strict thread matches above, loose matches on Subject: below --
2014-11-03 20:42 HMM (heterogeneous memory management) v5 j.glisse
2014-11-03 20:42 ` [PATCH 3/5] lib: lockless generic and arch independent page table (gpt) v2 j.glisse
2014-11-06 22:32 ` Rik van Riel
2014-11-06 22:40 ` Jerome Glisse
2014-11-06 22:56 ` Rik van Riel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+55aFwxdT+G26O3mSmrMuq5f3Tf9cRVhOLxR7kpXZu49x8c5Q@mail.gmail.com \
--to=torvalds@linux-foundation.org \
--cc=Alexander.Deucher@amd.com \
--cc=Greg.Stoner@amd.com \
--cc=John.Bridgman@amd.com \
--cc=Laurent.Morichetti@amd.com \
--cc=Michael.Mantor@amd.com \
--cc=Oded.Gabbay@amd.com \
--cc=Paul.Blinzer@amd.com \
--cc=SCheung@nvidia.com \
--cc=aarcange@redhat.com \
--cc=airlied@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=arvindg@nvidia.com \
--cc=ben.sander@amd.com \
--cc=blc@redhat.com \
--cc=cabuschardt@nvidia.com \
--cc=dpoole@nvidia.com \
--cc=hpa@zytor.com \
--cc=j.glisse@gmail.com \
--cc=jdonohue@redhat.com \
--cc=jglisse@redhat.com \
--cc=jhubbard@nvidia.com \
--cc=joro@8bytes.org \
--cc=jweiner@redhat.com \
--cc=ldunning@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=liranl@mellanox.com \
--cc=lwoodman@redhat.com \
--cc=mgorman@suse.de \
--cc=mhairgrove@nvidia.com \
--cc=peterz@infradead.org \
--cc=raindel@mellanox.com \
--cc=riel@redhat.com \
--cc=roland@purestorage.com \
--cc=sgutti@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox