Re: [PATCH 1/2] Introduce copy_user_handle_tail routine

From: Andi Kleen
Date: Wed Jul 02 2008 - 11:07:00 EST



[again with correct ccs sorry]

Vitaly Mayatskikh wrote:
> > Andi Kleen <andi@xxxxxxxxxxxxxx> 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



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/