From: Linus Torvalds <torvalds@linux-foundation.org> To: Dan Williams <dan.j.williams@intel.com> Cc: "Luck, Tony" <tony.luck@intel.com>, Andy Lutomirski <luto@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Borislav Petkov <bp@alien8.de>, stable <stable@vger.kernel.org>, the arch/x86 maintainers <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>, Paul Mackerras <paulus@samba.org>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Erwin Tsaur <erwin.tsaur@intel.com>, Michael Ellerman <mpe@ellerman.id.au>, Arnaldo Carvalho de Melo <acme@kernel.org>, linux-nvdimm <linux-nvdimm@lists.01.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 0/2] Replace and improve "mcsafe" with copy_safe() Date: Thu, 30 Apr 2020 17:10:15 -0700 [thread overview] Message-ID: <CAHk-=wiMs=A90np0Hv5WjHY8HXQWpgtuq-xrrJvyk7_pNB4meg@mail.gmail.com> (raw) In-Reply-To: <CAPcyv4g0a406X9-=NATJZ9QqObim9Phdkb_WmmhsT9zvXsGSpw@mail.gmail.com> On Thu, Apr 30, 2020 at 4:52 PM Dan Williams <dan.j.williams@intel.com> wrote: > > You had me until here. Up to this point I was grokking that Andy's > "_fallible" suggestion does help explain better than "_safe", because > the copy is doing extra safety checks. copy_to_user() and > copy_to_user_fallible() mean *something* where copy_to_user_safe() > does not. It's a horrible word, btw. The word doesn't actually mean what Andy means it to mean. "fallible" means "can make mistakes", not "can fault". So "fallible" is a horrible name. But anyway, I don't hate something like "copy_to_user_fallible()" conceptually. The naming needs to be fixed, in that "user" can always take a fault, so it's the _source_ that can fault, not the "user" part. It was the "copy_safe()" model that I find unacceptable, that uses _one_ name for what is at the very least *four* different operations: - copy from faulting memory to user - copy from faulting memory to kernel - copy from kernel to faulting memory - copy within faulting memory No way can you do that with one single function. A kernel address and a user address may literally have the exact same bit representation. So the user vs kernel distinction _has_ to be in the name. The "kernel vs faulting" doesn't necessarily have to be there from an implementation standpoint, but it *should* be there, because - it might affect implemmentation - but even if it DOESN'T affect implementation, it should be separate just from the standpoint of being self-documenting code. > However you lose me on this "broken nvdimm semantics" contention. > There is nothing nvdimm-hardware specific about the copy_safe() > implementation, zero, nada, nothing new to the error model that DRAM > did not also inflict on the Linux implementation. Ok, so good. Let's kill this all, and just use memcpy(), and copy_to_user(). Just make sure that the nvdimm code doesn't use invalid kernel addresses or other broken poisoning. Problem solved. You can't have it both ways. Either memcpy just works, or it doesn't. So which way is it? Linus _______________________________________________ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org> To: Dan Williams <dan.j.williams@intel.com> Cc: "Luck, Tony" <tony.luck@intel.com>, Andy Lutomirski <luto@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Borislav Petkov <bp@alien8.de>, stable <stable@vger.kernel.org>, "the arch/x86 maintainers" <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>, Paul Mackerras <paulus@samba.org>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Erwin Tsaur <erwin.tsaur@intel.com>, Michael Ellerman <mpe@ellerman.id.au>, Arnaldo Carvalho de Melo <acme@kernel.org>, linux-nvdimm <linux-nvdimm@lists.01.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 0/2] Replace and improve "mcsafe" with copy_safe() Date: Thu, 30 Apr 2020 17:10:15 -0700 [thread overview] Message-ID: <CAHk-=wiMs=A90np0Hv5WjHY8HXQWpgtuq-xrrJvyk7_pNB4meg@mail.gmail.com> (raw) In-Reply-To: <CAPcyv4g0a406X9-=NATJZ9QqObim9Phdkb_WmmhsT9zvXsGSpw@mail.gmail.com> On Thu, Apr 30, 2020 at 4:52 PM Dan Williams <dan.j.williams@intel.com> wrote: > > You had me until here. Up to this point I was grokking that Andy's > "_fallible" suggestion does help explain better than "_safe", because > the copy is doing extra safety checks. copy_to_user() and > copy_to_user_fallible() mean *something* where copy_to_user_safe() > does not. It's a horrible word, btw. The word doesn't actually mean what Andy means it to mean. "fallible" means "can make mistakes", not "can fault". So "fallible" is a horrible name. But anyway, I don't hate something like "copy_to_user_fallible()" conceptually. The naming needs to be fixed, in that "user" can always take a fault, so it's the _source_ that can fault, not the "user" part. It was the "copy_safe()" model that I find unacceptable, that uses _one_ name for what is at the very least *four* different operations: - copy from faulting memory to user - copy from faulting memory to kernel - copy from kernel to faulting memory - copy within faulting memory No way can you do that with one single function. A kernel address and a user address may literally have the exact same bit representation. So the user vs kernel distinction _has_ to be in the name. The "kernel vs faulting" doesn't necessarily have to be there from an implementation standpoint, but it *should* be there, because - it might affect implemmentation - but even if it DOESN'T affect implementation, it should be separate just from the standpoint of being self-documenting code. > However you lose me on this "broken nvdimm semantics" contention. > There is nothing nvdimm-hardware specific about the copy_safe() > implementation, zero, nada, nothing new to the error model that DRAM > did not also inflict on the Linux implementation. Ok, so good. Let's kill this all, and just use memcpy(), and copy_to_user(). Just make sure that the nvdimm code doesn't use invalid kernel addresses or other broken poisoning. Problem solved. You can't have it both ways. Either memcpy just works, or it doesn't. So which way is it? Linus
next prev parent reply other threads:[~2020-05-01 0:17 UTC|newest] Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-30 8:24 [PATCH v2 0/2] Replace and improve "mcsafe" with copy_safe() Dan Williams 2020-04-30 8:24 ` Dan Williams 2020-04-30 8:25 ` [PATCH v2 1/2] copy_safe: Rename memcpy_mcsafe() to copy_safe() Dan Williams 2020-04-30 8:25 ` Dan Williams 2020-05-01 2:55 ` Sasha Levin 2020-04-30 8:25 ` [PATCH v2 2/2] x86/copy_safe: Introduce copy_safe_fast() Dan Williams 2020-04-30 8:25 ` Dan Williams 2020-05-01 2:55 ` Sasha Levin 2020-04-30 14:02 ` [PATCH v2 0/2] Replace and improve "mcsafe" with copy_safe() Linus Torvalds 2020-04-30 14:02 ` Linus Torvalds 2020-04-30 16:51 ` Andy Lutomirski 2020-04-30 16:51 ` Andy Lutomirski 2020-04-30 17:17 ` Linus Torvalds 2020-04-30 17:17 ` Linus Torvalds 2020-04-30 18:42 ` Andy Lutomirski 2020-04-30 18:42 ` Andy Lutomirski 2020-04-30 19:22 ` Luck, Tony 2020-04-30 19:22 ` Luck, Tony 2020-04-30 19:50 ` Linus Torvalds 2020-04-30 19:50 ` Linus Torvalds 2020-04-30 20:25 ` Luck, Tony 2020-04-30 20:25 ` Luck, Tony 2020-04-30 23:52 ` Dan Williams 2020-04-30 23:52 ` Dan Williams 2020-05-01 0:10 ` Linus Torvalds [this message] 2020-05-01 0:10 ` Linus Torvalds 2020-05-01 0:23 ` Andy Lutomirski 2020-05-01 0:23 ` Andy Lutomirski 2020-05-01 0:39 ` Linus Torvalds 2020-05-01 0:39 ` Linus Torvalds 2020-05-01 1:10 ` Andy Lutomirski 2020-05-01 1:10 ` Andy Lutomirski 2020-05-01 14:09 ` Luck, Tony 2020-05-01 14:09 ` Luck, Tony 2020-05-03 0:29 ` Andy Lutomirski 2020-05-03 0:29 ` Andy Lutomirski 2020-05-04 20:05 ` Luck, Tony 2020-05-04 20:05 ` Luck, Tony 2020-05-04 20:26 ` Andy Lutomirski 2020-05-04 20:26 ` Andy Lutomirski 2020-05-04 21:30 ` Dan Williams 2020-05-04 21:30 ` Dan Williams 2020-05-01 0:24 ` Linus Torvalds 2020-05-01 0:24 ` Linus Torvalds 2020-05-01 1:20 ` Andy Lutomirski 2020-05-01 1:20 ` Andy Lutomirski 2020-05-01 1:21 ` Dan Williams 2020-05-01 1:21 ` Dan Williams 2020-05-01 18:28 ` Linus Torvalds 2020-05-01 18:28 ` Linus Torvalds 2020-05-01 20:17 ` Dave Hansen 2020-05-01 20:17 ` Dave Hansen 2020-05-03 12:57 ` David Laight 2020-05-03 12:57 ` David Laight 2020-05-04 18:33 ` Dan Williams 2020-05-04 18:33 ` Dan Williams 2020-05-11 15:24 ` Vivek Goyal 2020-05-11 15:24 ` Vivek Goyal 2020-04-30 19:51 ` Dan Williams 2020-04-30 19:51 ` Dan Williams 2020-04-30 20:07 ` Andy Lutomirski 2020-04-30 20:07 ` Andy Lutomirski 2020-05-01 7:46 ` David Laight 2020-05-01 7:46 ` David Laight
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHk-=wiMs=A90np0Hv5WjHY8HXQWpgtuq-xrrJvyk7_pNB4meg@mail.gmail.com' \ --to=torvalds@linux-foundation.org \ --cc=acme@kernel.org \ --cc=benh@kernel.crashing.org \ --cc=bp@alien8.de \ --cc=dan.j.williams@intel.com \ --cc=erwin.tsaur@intel.com \ --cc=hpa@zytor.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.org \ --cc=luto@kernel.org \ --cc=mingo@redhat.com \ --cc=mpe@ellerman.id.au \ --cc=paulus@samba.org \ --cc=peterz@infradead.org \ --cc=stable@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=tony.luck@intel.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.