| Message ID | 160087928642.3520.17063139768910633998.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Renovate memcpy_mcsafe with copy_mc_to_{user, kernel} | expand |
On Wed, Sep 23, 2020 at 10:00 AM Dan Williams <dan.j.williams@intel.com> wrote: > > Changes since v8 [1]: > - Rebase on v5.9-rc6 > > - Fix a performance regression in the x86 copy_mc_to_user() > implementation that was duplicating copies in the "fragile" case. > > - Refreshed the cover letter. > > [1]: http://lore.kernel.org/r/159630255616.3143511.18110575960499749012.stgit@dwillia2-desk3.amr.corp.intel.com > Given that Linus was the primary source of review feedback on these patches and a version of them have been soaking in -next with only a minor conflict report with vfs.git for the entirety of the v5.9-rc cycle (left there inadvertently while I was on leave), any concerns with me sending this to Linus directly during the merge window? > > The motivations to go rework memcpy_mcsafe() are that the benefit of > doing slow and careful copies is obviated on newer CPUs, and that the > current opt-in list of cpus to instrument recovery is broken relative to > those cpus. There is no need to keep an opt-in list up to date on an > ongoing basis if pmem/dax operations are instrumented for recovery by > default. With recovery enabled by default the old "mcsafe_key" opt-in to > careful copying can be made a "fragile" opt-out. Where the "fragile" > list takes steps to not consume poison across cachelines. > > The discussion with Linus made clear that the current "_mcsafe" suffix > was imprecise to a fault. The operations that are needed by pmem/dax are > to copy from a source address that might throw #MC to a destination that > may write-fault, if it is a user page. So copy_to_user_mcsafe() becomes > copy_mc_to_user() to indicate the separate precautions taken on source > and destination. copy_mc_to_kernel() is introduced as a version that > does not expect write-faults on the destination, but is still prepared > to abort with an error code upon taking #MC. > > These patches have received a kbuild-robot build success notification > across 114 configs, the rebase to v5.9-rc6 did not encounter any > conflicts, and the merge with tip/master is conflict-free. > > --- > > Dan Williams (2): > x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user,kernel}() > x86/copy_mc: Introduce copy_mc_generic() > > > arch/powerpc/Kconfig | 2 > arch/powerpc/include/asm/string.h | 2 > arch/powerpc/include/asm/uaccess.h | 40 +++-- > arch/powerpc/lib/Makefile | 2 > arch/powerpc/lib/copy_mc_64.S | 4 > arch/x86/Kconfig | 2 > arch/x86/Kconfig.debug | 2 > arch/x86/include/asm/copy_mc_test.h | 75 +++++++++ > arch/x86/include/asm/mcsafe_test.h | 75 --------- > arch/x86/include/asm/string_64.h | 32 ---- > arch/x86/include/asm/uaccess.h | 21 +++ > arch/x86/include/asm/uaccess_64.h | 20 -- > arch/x86/kernel/cpu/mce/core.c | 8 - > arch/x86/kernel/quirks.c | 9 - > arch/x86/lib/Makefile | 1 > arch/x86/lib/copy_mc.c | 65 ++++++++ > arch/x86/lib/copy_mc_64.S | 165 ++++++++++++++++++++ > arch/x86/lib/memcpy_64.S | 115 -------------- > arch/x86/lib/usercopy_64.c | 21 --- > drivers/md/dm-writecache.c | 15 +- > drivers/nvdimm/claim.c | 2 > drivers/nvdimm/pmem.c | 6 - > include/linux/string.h | 9 - > include/linux/uaccess.h | 9 + > include/linux/uio.h | 10 + > lib/Kconfig | 7 + > lib/iov_iter.c | 43 +++-- > tools/arch/x86/include/asm/mcsafe_test.h | 13 -- > tools/arch/x86/lib/memcpy_64.S | 115 -------------- > tools/objtool/check.c | 5 - > tools/perf/bench/Build | 1 > tools/perf/bench/mem-memcpy-x86-64-lib.c | 24 --- > tools/testing/nvdimm/test/nfit.c | 48 +++--- > .../testing/selftests/powerpc/copyloops/.gitignore | 2 > tools/testing/selftests/powerpc/copyloops/Makefile | 6 - > .../selftests/powerpc/copyloops/copy_mc_64.S | 1 > .../selftests/powerpc/copyloops/memcpy_mcsafe_64.S | 1 > 37 files changed, 452 insertions(+), 526 deletions(-) > rename arch/powerpc/lib/{memcpy_mcsafe_64.S => copy_mc_64.S} (98%) > create mode 100644 arch/x86/include/asm/copy_mc_test.h > delete mode 100644 arch/x86/include/asm/mcsafe_test.h > create mode 100644 arch/x86/lib/copy_mc.c > create mode 100644 arch/x86/lib/copy_mc_64.S > delete mode 100644 tools/arch/x86/include/asm/mcsafe_test.h > delete mode 100644 tools/perf/bench/mem-memcpy-x86-64-lib.c > create mode 120000 tools/testing/selftests/powerpc/copyloops/copy_mc_64.S > delete mode 120000 tools/testing/selftests/powerpc/copyloops/memcpy_mcsafe_64.S > > base-commit: ba4f184e126b751d1bffad5897f263108befc780 > _______________________________________________ > Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org > To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
On Tue, Sep 29, 2020 at 03:32:07PM -0700, Dan Williams wrote: > Given that Linus was the primary source of review feedback on these > patches and a version of them have been soaking in -next with only a > minor conflict report with vfs.git for the entirety of the v5.9-rc > cycle (left there inadvertently while I was on leave), any concerns > with me sending this to Linus directly during the merge window? What's wrong with them going through tip? But before that pls have a look at this question I have here: https://lkml.kernel.org/r/20200929102512.GB21110@zn.tnic Thx.
On Tue, Sep 29, 2020 at 10:05 PM Borislav Petkov <bp@alien8.de> wrote: > > On Tue, Sep 29, 2020 at 03:32:07PM -0700, Dan Williams wrote: > > Given that Linus was the primary source of review feedback on these > > patches and a version of them have been soaking in -next with only a > > minor conflict report with vfs.git for the entirety of the v5.9-rc > > cycle (left there inadvertently while I was on leave), any concerns > > with me sending this to Linus directly during the merge window? > > What's wrong with them going through tip? There's been a paucity of response on these after converging on the feedback from Linus. They missed v5.9, and I started casting about for what could be done to make sure they did not also miss v5.10 if the quiet continued. The preference is still "through tip". > But before that pls have a look at this question I have here: > > https://lkml.kernel.org/r/20200929102512.GB21110@zn.tnic > > Thx. Thanks, Boris, will do.
On Wed, Sep 30, 2020 at 08:49:42AM -0700, Dan Williams wrote: > There's been a paucity of response on these after converging on the > feedback from Linus. They missed v5.9, and I started casting about for > what could be done to make sure they did not also miss v5.10 if the > quiet continued. The preference is still "through tip". Ok, I'll try to queue them but pls respin soonish. That is, if Linus cuts -rc8 we have plenty of time but he didn't sound 100% on the -rc8 thing. Thx.
On Wed, Sep 30, 2020 at 9:24 AM Borislav Petkov <bp@alien8.de> wrote: > > Ok, I'll try to queue them but pls respin soonish. That is, if Linus > cuts -rc8 we have plenty of time but he didn't sound 100% on the -rc8 > thing. Oh, it's pretty much 100%. I can't imagine what would make me skip an rc8 at this point. Everything looks good right now (but not rc7, we had a stupid bug), but I'd rather wait a week than fins another silly bug the day after release (like happened in rc7).. We're talking literal "biblical burning bushes telling me to do a release" kind of events to skip rc8 by now. Linus
On Wed, Sep 30, 2020 at 09:28:33AM -0700, Linus Torvalds wrote: > Oh, it's pretty much 100%. Oh good. > I can't imagine what would make me skip an rc8 at this point. > Everything looks good right now (but not rc7, we had a stupid bug), > but I'd rather wait a week than fins another silly bug the day after > release (like happened in rc7).. Yeah, -rc8 is clearly the best idea, why *wouldn't* one do it?! :-))) > We're talking literal "biblical burning bushes telling me to do a > release" kind of events to skip rc8 by now. If you do, it probably'll look like this: diff --git a/Makefile b/Makefile index 992d24467ca0..5e8819b99110 100644 --- a/Makefile +++ b/Makefile @@ -2,8 +2,8 @@ VERSION = 5 PATCHLEVEL = 9 SUBLEVEL = 0 -EXTRAVERSION = -rc7 -NAME = Kleptomaniac Octopus +EXTRAVERSION = +NAME = Biblical Burning Bushes # *DOCUMENTATION* # To see a list of typical targets execute "make help"
Changes since v8 [1]: - Rebase on v5.9-rc6 - Fix a performance regression in the x86 copy_mc_to_user() implementation that was duplicating copies in the "fragile" case. - Refreshed the cover letter. [1]: http://lore.kernel.org/r/159630255616.3143511.18110575960499749012.stgit@dwillia2-desk3.amr.corp.intel.com --- The motivations to go rework memcpy_mcsafe() are that the benefit of doing slow and careful copies is obviated on newer CPUs, and that the current opt-in list of cpus to instrument recovery is broken relative to those cpus. There is no need to keep an opt-in list up to date on an ongoing basis if pmem/dax operations are instrumented for recovery by default. With recovery enabled by default the old "mcsafe_key" opt-in to careful copying can be made a "fragile" opt-out. Where the "fragile" list takes steps to not consume poison across cachelines. The discussion with Linus made clear that the current "_mcsafe" suffix was imprecise to a fault. The operations that are needed by pmem/dax are to copy from a source address that might throw #MC to a destination that may write-fault, if it is a user page. So copy_to_user_mcsafe() becomes copy_mc_to_user() to indicate the separate precautions taken on source and destination. copy_mc_to_kernel() is introduced as a version that does not expect write-faults on the destination, but is still prepared to abort with an error code upon taking #MC. These patches have received a kbuild-robot build success notification across 114 configs, the rebase to v5.9-rc6 did not encounter any conflicts, and the merge with tip/master is conflict-free. --- Dan Williams (2): x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user,kernel}() x86/copy_mc: Introduce copy_mc_generic() arch/powerpc/Kconfig | 2 arch/powerpc/include/asm/string.h | 2 arch/powerpc/include/asm/uaccess.h | 40 +++-- arch/powerpc/lib/Makefile | 2 arch/powerpc/lib/copy_mc_64.S | 4 arch/x86/Kconfig | 2 arch/x86/Kconfig.debug | 2 arch/x86/include/asm/copy_mc_test.h | 75 +++++++++ arch/x86/include/asm/mcsafe_test.h | 75 --------- arch/x86/include/asm/string_64.h | 32 ---- arch/x86/include/asm/uaccess.h | 21 +++ arch/x86/include/asm/uaccess_64.h | 20 -- arch/x86/kernel/cpu/mce/core.c | 8 - arch/x86/kernel/quirks.c | 9 - arch/x86/lib/Makefile | 1 arch/x86/lib/copy_mc.c | 65 ++++++++ arch/x86/lib/copy_mc_64.S | 165 ++++++++++++++++++++ arch/x86/lib/memcpy_64.S | 115 -------------- arch/x86/lib/usercopy_64.c | 21 --- drivers/md/dm-writecache.c | 15 +- drivers/nvdimm/claim.c | 2 drivers/nvdimm/pmem.c | 6 - include/linux/string.h | 9 - include/linux/uaccess.h | 9 + include/linux/uio.h | 10 + lib/Kconfig | 7 + lib/iov_iter.c | 43 +++-- tools/arch/x86/include/asm/mcsafe_test.h | 13 -- tools/arch/x86/lib/memcpy_64.S | 115 -------------- tools/objtool/check.c | 5 - tools/perf/bench/Build | 1 tools/perf/bench/mem-memcpy-x86-64-lib.c | 24 --- tools/testing/nvdimm/test/nfit.c | 48 +++--- .../testing/selftests/powerpc/copyloops/.gitignore | 2 tools/testing/selftests/powerpc/copyloops/Makefile | 6 - .../selftests/powerpc/copyloops/copy_mc_64.S | 1 .../selftests/powerpc/copyloops/memcpy_mcsafe_64.S | 1 37 files changed, 452 insertions(+), 526 deletions(-) rename arch/powerpc/lib/{memcpy_mcsafe_64.S => copy_mc_64.S} (98%) create mode 100644 arch/x86/include/asm/copy_mc_test.h delete mode 100644 arch/x86/include/asm/mcsafe_test.h create mode 100644 arch/x86/lib/copy_mc.c create mode 100644 arch/x86/lib/copy_mc_64.S delete mode 100644 tools/arch/x86/include/asm/mcsafe_test.h delete mode 100644 tools/perf/bench/mem-memcpy-x86-64-lib.c create mode 120000 tools/testing/selftests/powerpc/copyloops/copy_mc_64.S delete mode 120000 tools/testing/selftests/powerpc/copyloops/memcpy_mcsafe_64.S base-commit: ba4f184e126b751d1bffad5897f263108befc780