Messages in this thread | | | Date | Fri, 3 Oct 2014 06:53:59 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 2/2] x86: Only do a single page fault for copy_from_user_nmi |
| |
* Andi Kleen <ak@linux.intel.com> wrote:
> > For now, changing the semantics of the function seems like a > > sure way to fail in the future though. > > I doubt it. Nearly nobody uses the exact return value > semantics. > > (iirc it's mostly write() and some bizarre code in mount) > > In fact it's a regular mistake to assume it returns -errno. > > > > In theory we could also duplicate the whole copy_*_ path > > > for cases where the caller doesn't care about the exact > > > bytes. But that seems overkill for just this issue, and I'm > > > not sure anyone else cares about how fast this is. The > > > simpler check works as well for now. > > > > So I don't get that code, but why not fix it in general? > > Taking two faults seems silly. > > It's really complicated to reconstruct the exact bytes, as an > optimized memcpy is very complicated and has a lot of corner > cases. > > I tried it originally when writing the original copy function, > but failed. That is why people came up later with this > two-fault scheme. > > I think two fault is fine for most cases, just not for NMIs.
Slapping an ugly in_nmi() check into a generic memcpy routine, especially if it changes semantics, is asking for trouble. It is a non-starter and I'm NAK-ing it.
There are cleaner ways to solve this problem - PeterZ offered one, but there are other options as well, such as:
- removing exact-bytes semantics explicitly from almost all cases and offering a separate (and more expensive, in the faulting case) memcpy variant for write() and other code that absolutely must know the number of copied bytes.
- or adding a special no-bytes-copied memcpy variant that the NMI code could use.
Use one of these or outline that they cannot possibly work.
It might be more work for you, but it gives us a cleaner and more maintainable kernel. The problem is that you should know this general principle already, instead you are wasting maintainer bandwidth via arguing in favor of ugly hacks again and again...
Thanks,
Ingo
| |