From: Andi Kleen <andi@firstfloor.org>
To: Vitaly Mayatskikh <v.mayatskih@gmail.com>
Cc: linux-kernel@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 1/2] Introduce copy_user_handle_tail routine
Date: Wed, 02 Jul 2008 17:06:47 +0200 [thread overview]
Message-ID: <486B9987.8000601@firstfloor.org> (raw)
In-Reply-To: <m37ic4jr2w.fsf@gravicappa.englab.brq.redhat.com>
[again with correct ccs sorry]
Vitaly Mayatskikh wrote:
> > Andi Kleen <andi@firstfloor.org> writes:
> >
>> >> get/put user are macros and it's normally not a good idea to use ++ in macro
>> >> arguments because they might expand multiple times.
>> >>
>> >> sizeof(char) is always 1
>> >>
>> >> Also hopefully there's no sign extension anywhere with the signed char
> >
> > I have tested it a lot. I don't know of any fail scenario at the moment.
> >
>> >> Overall you could write it much simpler with a rep ; movs I think,
>> >> like traditional linux did.
> >
> > rep movs can fail.
How? (if it's a byte copy?)
The old 2.4 copy_*_user always used to that and it worked just fine AFAIK.
>> >> Similar problem with ++
>> >>
>> >> If zerorest is ever 0 then retesting it on every iteration seems
>> >> somewhat dumb.
> >
> > If zerorest is 0, this cycle will never be executed.
Ok but when it's not then it will be executed on each iteration.
>> >> I think a simple memset would be actually ok, i don't think we ever zero
>> >> anything that faults. That would be obviously racy anyways. If the zero
>> >> are supposed to override something then a racing user thread could always
>> >> catch it.
> >
> > Linus wanted this routine to be extremely dumb. This is the reason why tail
> > handling was moved from assembly to C. Yeah, my original patches were in
> > assembly and on the top of your realization.
My point was that it could be simpler because zeroing should not ever fault
(copy_in_user is not supposed to zero)
-Andi
next prev parent reply other threads:[~2008-07-02 15:06 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 [this message]
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
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=486B9987.8000601@firstfloor.org \
--to=andi@firstfloor.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.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.