Re: [PATCH 2/2] x86: Only do a single page fault for copy_from_user_nmi

[Ingo Molnar <mingo@kernel.org>]

* 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