From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: [PATCH 1/8] kcsan: Add Kernel Concurrency Sanitizer infrastructure Date: Thu, 17 Oct 2019 09:47:30 +0200 Message-ID: <20191017074730.GW2328@hirez.programming.kicks-ass.net> References: <20191016083959.186860-1-elver@google.com> <20191016083959.186860-2-elver@google.com> <20191016184346.GT2328@hirez.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Marco Elver Cc: LKMM Maintainers -- Akira Yokosawa , Alan Stern , Alexander Potapenko , Andrea Parri , Andrey Konovalov , Andy Lutomirski , Ard Biesheuvel , Arnd Bergmann , Boqun Feng , Borislav Petkov , Daniel Axtens , Daniel Lustig , dave.hansen@linux.intel.com, David Howells , Dmitry Vyukov , "H. Peter Anvin" , Ingo Molnar , Jade Alglave , Joel Fernandes , Jonathan Corbet , Josh Poimboeuf List-Id: linux-arch.vger.kernel.org On Wed, Oct 16, 2019 at 09:34:05PM +0200, Marco Elver wrote: > On Wed, 16 Oct 2019 at 20:44, Peter Zijlstra wrote: > > > + /* > > > + * Disable interrupts & preemptions, to ignore races due to accesses in > > > + * threads running on the same CPU. > > > + */ > > > + local_irq_save(irq_flags); > > > + preempt_disable(); > > > > Is there a point to that preempt_disable() here? > > We want to avoid being preempted while the watchpoint is set up; > otherwise, we would report data-races for CPU-local data, which is > incorrect. Disabling IRQs already very much disables preemption. There is absolutely no point in doing preempt_disable() when the whole section already runs with IRQs disabled. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bombadil.infradead.org ([198.137.202.133]:52268 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387930AbfJQHr6 (ORCPT ); Thu, 17 Oct 2019 03:47:58 -0400 Date: Thu, 17 Oct 2019 09:47:30 +0200 From: Peter Zijlstra Subject: Re: [PATCH 1/8] kcsan: Add Kernel Concurrency Sanitizer infrastructure Message-ID: <20191017074730.GW2328@hirez.programming.kicks-ass.net> References: <20191016083959.186860-1-elver@google.com> <20191016083959.186860-2-elver@google.com> <20191016184346.GT2328@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Marco Elver Cc: LKMM Maintainers -- Akira Yokosawa , Alan Stern , Alexander Potapenko , Andrea Parri , Andrey Konovalov , Andy Lutomirski , Ard Biesheuvel , Arnd Bergmann , Boqun Feng , Borislav Petkov , Daniel Axtens , Daniel Lustig , dave.hansen@linux.intel.com, David Howells , Dmitry Vyukov , "H. Peter Anvin" , Ingo Molnar , Jade Alglave , Joel Fernandes , Jonathan Corbet , Josh Poimboeuf , Luc Maranget , Mark Rutland , Nicholas Piggin , "Paul E. McKenney" , Thomas Gleixner , Will Deacon , kasan-dev , linux-arch , "open list:DOCUMENTATION" , linux-efi@vger.kernel.org, Linux Kbuild mailing list , LKML , Linux Memory Management List , the arch/x86 maintainers Message-ID: <20191017074730.CPqPLj0TnJj5xsqxs7BrAigqF-jUaak-1fhXnWeOUg0@z> On Wed, Oct 16, 2019 at 09:34:05PM +0200, Marco Elver wrote: > On Wed, 16 Oct 2019 at 20:44, Peter Zijlstra wrote: > > > + /* > > > + * Disable interrupts & preemptions, to ignore races due to accesses in > > > + * threads running on the same CPU. > > > + */ > > > + local_irq_save(irq_flags); > > > + preempt_disable(); > > > > Is there a point to that preempt_disable() here? > > We want to avoid being preempted while the watchpoint is set up; > otherwise, we would report data-races for CPU-local data, which is > incorrect. Disabling IRQs already very much disables preemption. There is absolutely no point in doing preempt_disable() when the whole section already runs with IRQs disabled.