linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] x86/memcpy: Introduce memcpy_mcsafe_fast
@ 2020-04-18 20:30 Andy Lutomirski
  2020-04-18 20:52 ` Linus Torvalds
  0 siblings, 1 reply; 27+ messages in thread
From: Andy Lutomirski @ 2020-04-18 20:30 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Dan Williams, Thomas Gleixner, Ingo Molnar, X86 ML, stable,
	Borislav Petkov, H. Peter Anvin, Peter Zijlstra, Tony Luck,
	Erwin Tsaur, Linux Kernel Mailing List, linux-nvdimm



--Andy

> On Apr 18, 2020, at 12:42 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
>>> On Fri, Apr 17, 2020 at 5:12 PM Dan Williams <dan.j.williams@intel.com> wrote:
>>> 
>>> @@ -106,12 +108,10 @@ static __always_inline __must_check unsigned long
>>> memcpy_mcsafe(void *dst, const void *src, size_t cnt)
>>> {
>>> #ifdef CONFIG_X86_MCE
>>> -       i(static_branch_unlikely(&mcsafe_key))
>>> -               return __memcpy_mcsafe(dst, src, cnt);
>>> -       else
>>> +       if (static_branch_unlikely(&mcsafe_slow_key))
>>> +               return memcpy_mcsafe_slow(dst, src, cnt);
>>> #endif
>>> -               memcpy(dst, src, cnt);
>>> -       return 0;
>>> +       return memcpy_mcsafe_fast(dst, src, cnt);
>>> }
> 
> It strikes me that I see no advantages to making this an inline function at all.
> 
> Even for the good case - where it turns into just a memcpy because MCE
> is entirely disabled - it doesn't seem to matter.
> 
> The only case that really helps is when the memcpy can be turned into
> a single access. Which - and I checked - does exist, with people doing
> 
>        r = memcpy_mcsafe(&sb_seq_count, &sb(wc)->seq_count, sizeof(uint64_t));
> 
> to read a single 64-bit field which looks aligned to me.
> 
> But that code is incredible garbage anyway, since even on a broken
> machine, there's no actual reason to use the slow variant for that
> whole access that I can tell. The macs-safe copy routines do not do
> anything worthwhile for a single access.

Maybe I’m missing something obvious, but what’s the alternative?  The _mcsafe variants don’t just avoid the REP mess — they also tell the kernel that this particular access is recoverable via extable. With a regular memory access, the CPU may not explode, but do_machine_check() will, at very best, OOPS, and even that requires a certain degree of optimism.  A panic is more likely.

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2020-10-08 17:08 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-18 20:30 [PATCH] x86/memcpy: Introduce memcpy_mcsafe_fast Andy Lutomirski
2020-04-18 20:52 ` Linus Torvalds
2020-04-20  5:08   ` Dan Williams
2020-04-20 17:28     ` Linus Torvalds
2020-04-20 18:20       ` Dan Williams
2020-04-20 19:05         ` Linus Torvalds
2020-04-20 19:29           ` Dan Williams
2020-04-20 20:07             ` Linus Torvalds
2020-04-20 20:23               ` Luck, Tony
2020-04-20 20:27                 ` Linus Torvalds
2020-04-20 20:45                   ` Luck, Tony
2020-04-20 20:56                     ` Linus Torvalds
2020-04-20 20:24               ` Dan Williams
2020-04-20 20:46                 ` Linus Torvalds
2020-04-20 20:57                   ` Luck, Tony
2020-04-20 21:16                     ` Linus Torvalds
2020-10-06  9:57       ` [tip: ras/core] x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}() tip-bot2 for Dan Williams
2020-10-07 11:14         ` Borislav Petkov
2020-10-07 16:45           ` Borislav Petkov
2020-10-07 17:03             ` Borislav Petkov
2020-10-07 18:53               ` Dan Williams
2020-10-07 19:25                 ` Borislav Petkov
2020-10-08 16:59                   ` Dan Williams
2020-10-08 17:08                     ` Borislav Petkov
2020-10-07 17:51             ` Dan Williams
2020-10-07 18:24           ` [PATCH] x86/mce: Gate copy_mc_fragile() export by CONFIG_COPY_MC_TEST=y Dan Williams
2020-10-08  9:01           ` [tip: ras/core] x86/mce: Allow for copy_mc_fragile symbol checksum to be generated tip-bot2 for Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).