* Linus Torvalds <torva...@linux-foundation.org> wrote:
> On Sat, Mar 12, 2016 at 9:16 AM, Ingo Molnar <mi...@kernel.org> wrote:
> >
> > Please use the copy_*_user() memory copying API semantics, which are: return
> > negative code (-EFAULT) on error, 0 on success.
>
> Those are the get_user/put_user() semantics.
>
> copy_*_user() has those annoying "bytes left uncopied" return values
> that I really wouldn't encourage anybody else use unless they really
> have to. [...]
Indeed, and I even grepped for it because I tend to get it wrong ... I suck!
> [...] There are (good) reasons why it's done that way, but it's still not
> something that should be aped without the same kind of major reasons.
Yeah, so IIRC the main reason being security and the ability to know which bits
of
a possibly uninitialized area are not yet copied on.
But with primary memcpy_mcsafe() usage being in-kernel (MM)IO space management
I
don't think we have the 'uninitialized' problem in nearly as marked a fashion,
and
we very much have the (largely uncompensated) cost of more complex assembly.
Nevertheless, wrt. error handling, the bit I feel very strongly about is the
following:
We have the existing 'mempcy API check for failure' pattern of:
if (put_user()) {
... it didn't succeed ...
}
if (copy_from_user()) {
... it didn't succeed ...
}
... which should be kept IMHO:
if (memcpy_mcsafe()) {
... it didn't succeed ...
}
... and what I most disagree with most is one of the suggestions in the
original
mail I replied to:
>> 2) Reverse the return values.
(== return 1 on success, 0 on failure)
... that's crazy IMHO and reverses the pattern of almost every
memcpy-with-check
(or syscall-success) API we have!
So I still think we should have the regular '0 on success, negative error value
on
failure' semantics - but I'd not be hugely against a '0 on success, bytes left
uncopied on failure' variant either in principle, except that I think it unduly
complicates the assembly side to recover the bytes.
The '0/1' current implementation is really neither of these but still fits the
common error checking pattern, but it's still much better than the '1/0'
suggestion which is crazy!
So I think we should move from 0/1 to 0/-EFAULT for now, and maybe (maybe) move
to
a 0/bytes_left return code in the future, if there comes a strong usecase. Most
error checks will have to be converted in that case anyway to make use of the
return code, so it's not like we introduce an extra cost here by going to
0/-EFAULT.
Also, by going to 0/-EFAULT we have the option to also allow other negative
error
codes, which would allow the distinction of #MC traps from regular page fault
traps for example. (a driver might want to log them differently.)
Thanks,
Ingo
