All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Ingo Molnar <mingo@redhat.com>
Cc: Tony Luck <tony.luck@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Borislav Petkov <bp@alien8.de>, stable <stable@vger.kernel.org>,
	X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andy Lutomirski <luto@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Erwin Tsaur <erwin.tsaur@intel.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Mikulas Patocka <mpatocka@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	0day robot <lkp@intel.com>,
	linux-nvdimm <linux-nvdimm@lists.01.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jan Kara <jack@suse.cz>
Subject: Re: [PATCH v9 0/2] Renovate memcpy_mcsafe with copy_mc_to_{user, kernel}
Date: Tue, 29 Sep 2020 15:32:07 -0700	[thread overview]
Message-ID: <CAPcyv4iPuRWSv_do_h8stU0-SiWxtKkQWvzBEU+78fDE6VffmA@mail.gmail.com> (raw)
In-Reply-To: <160087928642.3520.17063139768910633998.stgit@dwillia2-desk3.amr.corp.intel.com>

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
_______________________________________________
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: Dan Williams <dan.j.williams@intel.com>
To: Ingo Molnar <mingo@redhat.com>
Cc: Tony Luck <tony.luck@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Borislav Petkov <bp@alien8.de>, stable <stable@vger.kernel.org>,
	X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andy Lutomirski <luto@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Erwin Tsaur <erwin.tsaur@intel.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Mikulas Patocka <mpatocka@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	0day robot <lkp@intel.com>,
	linux-nvdimm <linux-nvdimm@lists.01.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jan Kara <jack@suse.cz>
Subject: Re: [PATCH v9 0/2] Renovate memcpy_mcsafe with copy_mc_to_{user, kernel}
Date: Tue, 29 Sep 2020 15:32:07 -0700	[thread overview]
Message-ID: <CAPcyv4iPuRWSv_do_h8stU0-SiWxtKkQWvzBEU+78fDE6VffmA@mail.gmail.com> (raw)
In-Reply-To: <160087928642.3520.17063139768910633998.stgit@dwillia2-desk3.amr.corp.intel.com>

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

  parent reply	other threads:[~2020-09-29 22:32 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-23 16:41 [PATCH v9 0/2] Renovate memcpy_mcsafe with copy_mc_to_{user, kernel} Dan Williams
2020-09-23 16:41 ` Dan Williams
2020-09-23 16:41 ` [PATCH v9 1/2] x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}() Dan Williams
2020-09-23 16:41   ` Dan Williams
2020-09-29 10:25   ` Borislav Petkov
2020-09-29 10:25     ` Borislav Petkov
2020-09-30 16:13     ` Dan Williams
2020-09-30 16:13       ` Dan Williams
2020-09-23 16:42 ` [PATCH v9 2/2] x86/copy_mc: Introduce copy_mc_generic() Dan Williams
2020-09-23 16:42   ` Dan Williams
2020-09-29 22:32 ` Dan Williams [this message]
2020-09-29 22:32   ` [PATCH v9 0/2] Renovate memcpy_mcsafe with copy_mc_to_{user, kernel} Dan Williams
2020-09-30  5:04   ` Borislav Petkov
2020-09-30  5:04     ` Borislav Petkov
2020-09-30 15:49     ` Dan Williams
2020-09-30 15:49       ` Dan Williams
2020-09-30 16:24       ` Borislav Petkov
2020-09-30 16:24         ` Borislav Petkov
2020-09-30 16:28         ` Linus Torvalds
2020-09-30 16:28           ` Linus Torvalds
2020-09-30 16:38           ` Borislav Petkov
2020-09-30 16:38             ` Borislav Petkov

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=CAPcyv4iPuRWSv_do_h8stU0-SiWxtKkQWvzBEU+78fDE6VffmA@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=acme@kernel.org \
    --cc=benh@kernel.crashing.org \
    --cc=bp@alien8.de \
    --cc=erwin.tsaur@intel.com \
    --cc=hpa@zytor.com \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=lkp@intel.com \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=mpatocka@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=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --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: link
Be 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.