All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Anton Arapov <aarapov@redhat.com>
Cc: Vitaly Mayatskikh <v.mayatskih@gmail.com>,
	Andi Kleen <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] Fix copy_user on x86_64
Date: Thu, 26 Jun 2008 08:37:49 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0806260837290.10755@hp.linux-foundation.org> (raw)
In-Reply-To: <48635DA0.80102@redhat.com>


On Thu, 26 Jun 2008, Anton Arapov wrote:
> 
> This is the patch patch for copy_user routine, you've discussed recently.

I don't think it works right.

Isn't this same routine also used for copy_in_user()? For that case both 
source _and_ destination can fault, but your fixup routines assume that 
onle one of them does (ie the fixup for a load-fault does a store for the 
previously loaded valies, and assumes that it doesn't trap)

Also, I'd realy rather do this all by handling the "taul" case in C. We 
already effectively have _half_ that support: the "clear end" flag ends up 
calling our specialized memset() routine, but it would be much nicer if 
we:

 - extended the "clear end" flag to be not just "clear end", but also 
   which direction things are going.
 - always call a (fixed) fixup-routine that is written in C (because 
   performance on a cycle basis no longer matters) that gets the remaining 
   length and the source and destination as arguments, along with the 
   "clear and direction flag".
 - make that fixup routine do the byte-exact tests and any necessary 
   clearing (and return the possibly-fixed-up remaining length).

Notice how this way we still have _optimal_ performance for the case where 
no fault happens, and we don't need any complex fixups in assembly code at 
all - the only thing the asm routines need to do is to get the right 
length (we already have this) and fix up the source/dest pointers (we 
don't generally have this, although the zero-at-end fixes up the 
destination pointer in order to zero it, of course).

Hmm?

			Linus

  reply	other threads:[~2008-06-26 15:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-25 12:31 [PATCH] Fix copy_user on x86_64 Vitaly Mayatskikh
2008-06-26  9:13 ` Anton Arapov
2008-06-26 15:37   ` Linus Torvalds [this message]
2008-06-26 15:58     ` Vitaly Mayatskikh
2008-06-26 16:30       ` Linus Torvalds

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.0806260837290.10755@hp.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=aarapov@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.