On Thu, Apr 30, 2020 at 12:23 PM Luck, Tony <tony.l...@intel.com> wrote: > > How about > > try_copy_catch(void *dst, void *src, size_t count, int *fault) > > returns number of bytes not-copied (like copy_to_user etc). > > if return is not zero, "fault" tells you what type of fault > cause the early stop (#PF, #MC).
That just makes things worse. The problem isn't "what fault did I get?". The problem is "what is the point of this function?". In other words, I want the code to explain _why_ it uses a particular function. So the question the function should answer is not "Why did it take a fault?", but "Why isn't this just a 'memcpy()'"? When somebody writes x = copy_to_user(a,b,c); the "why is it not a memcpy" question never comes up, because the code is obvious. It's not a memory copy, because it's copying to user space, and user space is different! In contrast, if you write x = copy_safe(a,b,c); then what is going on? There is no rhyme or reason to the call. Is the source unsafe? Wny? Is the destination faulting? Why? Both? How? So "copy_to_user()" _answers_ a question. But "copy_safe()" just results in more questions. See the difference? And notice that the "why did it fault" question is NOT about your "what kind of fault did it take" question. That question is generally pretty much uninteresting. The question I want answered is "why is this function being called AT ALL". Again, "copy_to_user()" can fail, and we know to check failure cases. But more importantly, the _reason_ it can fail is obvious from the name and from the use. There's no confusion about "why". "copy_safe()"? or "try_copy_catch()"? No such thing. It doesn't answer that fundamental "why" question. And yes, this also has practical consequences. If you know that the failure is due to the source being some special memory (and if we care about the MC case with unaligned accesses), then the function in question should probably strive to make those _source_ accesses be the aligned ones. But if it's the destination that is special memory, then it's the writes to the destination that should be aligned. If you need both, you may need to be either mutually aligned, or do byte accesses, or do special shifting copies. So it matters for any initial alignment code (if the thing has alignment issues to begin with). I haven't even gotten an answer to the "why would the write fail". When I asked, Dan said "writes can mmu-fault now that memcpy_mcsafe() is also used by _copy_to_iter_mcsafe()" but as mentioned, the only reason I can find for _that_ is that the destination is user space. In which case a "copy_safe()" absolutely could never work. If I can't figure out the "why is this function used" or even figure out why it needs the semantics it claims it needs, then there's a problem. Personally, I suspect that the *easiest* way to support the broken nvdimm semantics is to not have a "copy" function at all as the basic accessor model. Why? Exactly because "copy" is not a fundamental well-defined action. You get nasty combinatorial explosions of different things, where you have three different kinds of sources (user, RAM, nvdimm) and three different kinds of destinations. And that's ignoring the whole "maybe you don't want to do a plain copy, maybe you want to calculate a RAID checksum, or do a 'strcpy()' or whatever". If those are ever issues, you get another explosion of combinations. The only *fundamental* access would likely be a single read/write operation, not a copy operation. Think "get_user()" instead of "copy_from_user()". Even there you get combinatorial explosions with access sizes, but you can often generate those automatically or with simple patterns, and then you can build up the copy functions from that if you really need to. Linus