All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] x86/mm/pat: silence a data race in cpa_4k_install
@ 2020-01-21 15:15 Qian Cai
  2020-01-21 15:19 ` Marco Elver
  0 siblings, 1 reply; 14+ messages in thread
From: Qian Cai @ 2020-01-21 15:15 UTC (permalink / raw)
  To: tglx, mingo, bp
  Cc: dave.hansen, luto, peterz, elver, x86, linux-kernel, Qian Cai

Macro Elver mentioned,

"Yes. I was finally able to reproduce this data race on linux-next (my
system doesn't crash though, maybe not enough cores?). Here is a trace
with line numbers:

read to 0xffffffffaa59a000 of 8 bytes by interrupt on cpu 7:
cpa_inc_4k_install arch/x86/mm/pat/set_memory.c:131 [inline]
__change_page_attr+0x10cf/0x1840 arch/x86/mm/pat/set_memory.c:1514
__change_page_attr_set_clr+0xce/0x490 arch/x86/mm/pat/set_memory.c:1636
__set_pages_np+0xc4/0xf0 arch/x86/mm/pat/set_memory.c:2148
__kernel_map_pages+0xb0/0xc8 arch/x86/mm/pat/set_memory.c:2178
kernel_map_pages include/linux/mm.h:2719 [inline]
<snip>

write to 0xffffffffaa59a000 of 8 bytes by task 1 on cpu 6:
cpa_inc_4k_install arch/x86/mm/pat/set_memory.c:131 [inline]
__change_page_attr+0x10ea/0x1840 arch/x86/mm/pat/set_memory.c:1514
__change_page_attr_set_clr+0xce/0x490 arch/x86/mm/pat/set_memory.c:1636
__set_pages_p+0xc4/0xf0 arch/x86/mm/pat/set_memory.c:2129
__kernel_map_pages+0x2e/0xc8 arch/x86/mm/pat/set_memory.c:2176
kernel_map_pages include/linux/mm.h:2719 [inline]
<snip>

Both accesses are due to the same "cpa_4k_install++" in
cpa_inc_4k_install. Now you can see that a data race here could be
potentially undesirable: depending on compiler optimizations or how
x86 executes a non-LOCK'd increment, you may lose increments, corrupt
the counter, etc. Since this counter only seems to be used for
printing some stats, this data race itself is unlikely to cause harm
to the system though."

This will generate a lot of noise on a debug kernel with debug_pagealloc
with KCSAN enabled which could render the system unusable. Silence it by
using the data_race() macro.

Suggested-by: Macro Elver <elver@google.com>
Signed-off-by: Qian Cai <cai@lca.pw>
---
 arch/x86/mm/pat/set_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 20823392f4f2..a5c35e57905e 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -128,7 +128,7 @@ static inline void cpa_inc_2m_checked(void)
 
 static inline void cpa_inc_4k_install(void)
 {
-	cpa_4k_install++;
+	data_race(cpa_4k_install++);
 }
 
 static inline void cpa_inc_lp_sameprot(int level)
-- 
2.21.0 (Apple Git-122.2)


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH -next] x86/mm/pat: silence a data race in cpa_4k_install
@ 2020-01-21 15:50 Qian Cai
  0 siblings, 0 replies; 14+ messages in thread
From: Qian Cai @ 2020-01-21 15:50 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Marco Elver, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra, the arch/x86 maintainers, LKML



> On Jan 21, 2020, at 10:45 AM, Borislav Petkov <bp@alien8.de> wrote:
> 
> Perhaps because you've been dealing with KCSAN for so long. :-)
> 
> The main angle here, IMO, is that this "fix" is being done solely for
> KCSAN. Or is there another reason to "fix" intentional data races? At
> least I don't see one. And the text says
> 
> "This will generate a lot of noise on a debug kernel with
> debug_pagealloc with KCSAN enabled which could render the system
> unusable."
> 
> So yes, I think it should say something about making KCSAN happy.
> 
> Oh, and while at it I'd prefer it if it did the __no_kcsan function
> annotation instead of the data_race() thing.

Or the patch title could be “play KCSAN well with debug_pagealloc”?

I am fine with __no_kcsan as well. I just need to retest the whole thing first.

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

end of thread, other threads:[~2020-01-23  8:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21 15:15 [PATCH -next] x86/mm/pat: silence a data race in cpa_4k_install Qian Cai
2020-01-21 15:19 ` Marco Elver
2020-01-21 15:28   ` Borislav Petkov
2020-01-21 15:33     ` Qian Cai
2020-01-21 15:36       ` Marco Elver
2020-01-21 15:45         ` Borislav Petkov
2020-01-21 20:21           ` Qian Cai
2020-01-21 22:18             ` Borislav Petkov
2020-01-21 23:30               ` Marco Elver
2020-01-22  0:34               ` Qian Cai
2020-01-22  8:46           ` Peter Zijlstra
2020-01-23  2:15             ` Qian Cai
2020-01-23  8:16               ` Borislav Petkov
2020-01-21 15:50 Qian Cai

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.