From: Linus Torvalds <torvalds@linux-foundation.org>
To: Vitaly Mayatskikh <v.mayatskih@gmail.com>
Cc: linux-kernel@vger.kernel.org, Andi Kleen <andi@firstfloor.org>,
Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH] x86: Optimize tail handling for copy_user
Date: Wed, 30 Jul 2008 10:29:32 -0700 (PDT) [thread overview]
Message-ID: <alpine.LFD.1.10.0807301016020.3334@nehalem.linux-foundation.org> (raw)
In-Reply-To: <m3ljzj60mk.fsf@gravicappa.englab.brq.redhat.com>
On Wed, 30 Jul 2008, Vitaly Mayatskikh wrote:
>
> Another try.
Ok, this is starting to look more reasonable. But you cannot split things
up like this per-file, because the end result doesn't _work_ with the
changes separated.
> BYTES_LEFT_IN_PAGE macro returns PAGE_SIZE, not zero, when the address
> is well aligned to page.
Hmm. Why? If the address is aligned, then we shouldn't even tro to copy
any more, should we? We know we got a fault - and regardless of whether it
was because of some offset off the base pointer or not, if the base
pointer was at offset zero, it's going to be in the same page. So why try
to do an operation we know will fault again?
Also, that's a rather inefficient way to do it, isn't it? Maybe the
compiler can figure it out, but the efficient code would be just
PAGE_SIZE - ((PAGE_SIZE-1) &(unsigned long)ptr)
no? That said, exactly because I think we shouldn't even bother to try to
fix up faults that happened at the beginning of a page, I think the right
one is the one I think I posted originally, ie the one that does just
#define BYTES_LEFT_IN_PAGE(ptr) \
(unsigned int)((PAGE_SIZE-1) & -(long)(ptr))
which is a bit simpler (well, it requires some thought to know why it
works, but it generates good code).
In case you wonder why it works, the operation we _want_ do do is
(PAGE_SIZE - offset-in-page) mod PAGE_SIZE
but subtraction is "stable" in modulus calculus (*), so you can write that
as
(PAGE_SIZE mod PAGE_SIZE - offset-in-page) mod PAGE_SIZE
which is just
(0 - (ptr mod PAGE_SIZE)) mod PAGE_SIZE
but again, subtraction is stable in modulus, so you can write that as
(0 - ptr) mod PAGE_SIZE
and so the result is literally just those single 'neg' and 'and'
instructions (in the macro, you then need all the casting and the
parenthesis, which is why it gets ugly again)
And yes, maybe the compiler figures it all out, but judging by past
experience, things often don't work that well.
Linus
(*) Yeah, in math, it's stable in general, in 2's complement arithmetic
it's only stable in mod 2^n, I guess.
next prev parent reply other threads:[~2008-07-30 17:33 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-28 14:10 Vitaly Mayatskikh
2008-07-28 15:48 ` Linus Torvalds
2008-07-29 15:24 ` Vitaly Mayatskikh
2008-07-30 12:02 ` Vitaly Mayatskikh
2008-07-30 17:29 ` Linus Torvalds [this message]
2008-07-31 14:27 ` Vitaly Mayatskikh
2008-08-01 20:10 ` Linus Torvalds
2008-08-01 22:04 ` Vitaly Mayatskikh
2008-08-01 22:30 ` Linus Torvalds
2008-08-01 22:53 ` Vitaly Mayatskikh
2008-07-30 12:07 ` Vitaly Mayatskikh
2008-07-30 12:13 ` Vitaly Mayatskikh
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=alpine.LFD.1.10.0807301016020.3334@nehalem.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=v.mayatskih@gmail.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
Powered by JetHome