mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Vitaly Mayatskikh <v.mayatskih@gmail.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 1/2] Introduce copy_user_handle_tail routine
Date: Mon, 7 Jul 2008 09:21:36 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0807070901490.11076@woody.linux-foundation.org> (raw)
In-Reply-To: <m34p71yjyx.fsf@gravicappa.englab.brq.redhat.com>



On Mon, 7 Jul 2008, Vitaly Mayatskikh wrote:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > Now, the stuff that comes *before* that point is the "try to fix up one 
> > byte at a time" thing, which I'd like to be simple and dumb. At least to 
> > start with.
> 
> Just to be clear: do these patches are good enough now (to start with)?
> Or, may be, it needs to be further improved?

I think they are getting there. I'm obviously not merging them in 2.6.26, 
but I'd be happy to do so for .27. 

Obviously, I'd be even happier if it also went through the normal x86 
review cycles (ie Ingo &co), but the current series is largely ack'ed by 
me.

> Btw, how much does it cost to CPU to do a fault? Can it be compared with
> average time of find_vma()?

It's *much* higher than a find_vma(). It's on the order of several 
thousand cycles, easy (well, it depends on uarch - on a P4, iirc any 
exception is soemthing like 1500 cycles *minimum*, and that's just for the 
exception overhead, not the actual fault path).

But the thing is, it doesn't even need a find_vma(). We can avoid the 
extra trap 99.9% of the time by knowing that the trap happened at a page 
crosser (in *theory* a trap can happen in the middle of a page because 
another CPU did a munmap() in the middle, but that's not a case we need to 
even bother optimize for). In particular, we *know* we shouldn't even try 
to cross user pages. So the fixup routine can just do

	/* Think about it.. */
	#define BYTES_LEFT_IN_PAGE(ptr) \
		(unsigned int)((PAGE_SIZE-1) & -(long)(ptr))


	/* How much should we try to copy carefully byte-by-byte? */
	unsigned int max_copy = remaining;

	/* Don't even bother trying to cross a page in user space! */
	if (flags & DEST_IS_USERSPACE)
		max_copy = min(max_copy, BYTES_LEFT_IN_PAGE(dst));
	if (flags & SOURCE_IS_USERSPACE)
		max_copy = min(max_copy, BYTES_LEFT_IN_PAGE(src));

	/* Do the careful copy */
	while (max_copy--) {
		unsigned char c;
		if (__get_user(c,src))
			break;
		if (__put_user(c,dst))
			break;
		src++;
		dst++;
		remaining--;
	}

	if (flags & CLEAR_REMAINDER)
		memset(dst, 0, remaining);

	return remaining;

or similar. Note how this still uses the slow-and-careful byte-at-a-time 
approach to the final copy, but it avoids - on purpose - even trying to 
copy across page boundaries, and thus will never take a second trap in the 
common case.

See? We don't actually care about vma boundaries or anything like that. We 
just care about the only boundary that matters for faults: the page 
boundary.

		Linus

  parent reply	other threads:[~2008-07-07 16:21 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-27 21:52 [PATCH 3/3] Fix copy_user on x86_64 Vitaly Mayatskikh
2008-06-28 18:26 ` Linus Torvalds
2008-06-30 15:12   ` Vitaly Mayatskikh
2008-06-30 15:55     ` Linus Torvalds
2008-06-30 16:16       ` Andi Kleen
2008-06-30 18:22       ` Kari Hurtta
2008-07-02 13:48       ` [PATCH 1/2] Introduce copy_user_handle_tail routine Vitaly Mayatskikh
2008-07-02 14:06         ` Andi Kleen
2008-07-02 14:31           ` Vitaly Mayatskikh
2008-07-02 15:06             ` Andi Kleen
2008-07-02 15:32               ` Vitaly Mayatskikh
2008-07-02 15:40                 ` Andi Kleen
2008-07-02 15:58                   ` Vitaly Mayatskikh
2008-07-02 18:54                     ` Andi Kleen
2008-07-03  2:35             ` Linus Torvalds
2008-07-07 12:09               ` Vitaly Mayatskikh
2008-07-07 12:12                 ` Vitaly Mayatskikh
2008-07-07 16:43                   ` Andi Kleen
2008-07-07 16:21                 ` Linus Torvalds [this message]
2008-07-07 17:05                   ` Vitaly Mayatskikh
2008-07-09 13:03                   ` Ingo Molnar
2008-07-09 13:16                     ` Vitaly Mayatskikh
2008-07-09 13:52                       ` Ingo Molnar
2008-07-02 13:53       ` [PATCH 2/2] Fix copy_user on x86 Vitaly Mayatskikh
2008-07-02 14:08         ` Andi Kleen
2008-07-02 14:36           ` 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.0807070901490.11076@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=linux-kernel@vger.kernel.org \
    --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

all inboxes | Powered by JetHome®