From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleg Nesterov Subject: Re: [PATCH v2 1/8] kcsan: Add Kernel Concurrency Sanitizer infrastructure Date: Wed, 23 Oct 2019 18:24:32 +0200 Message-ID: <20191023162432.GC14327@redhat.com> References: <20191017141305.146193-1-elver@google.com> <20191017141305.146193-2-elver@google.com> <20191022154858.GA13700@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline 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 , David Howells , Dmitry Vyukov , "H. Peter Anvin" , Ingo Molnar , Jade Alglave , Joel Fernandes , Jonathan Corbet List-Id: linux-arch.vger.kernel.org On 10/22, Marco Elver wrote: > > On Tue, 22 Oct 2019 at 17:49, Oleg Nesterov wrote: > > > > Just for example. Suppose that task->state =3D TASK_UNINTERRUPTIBLE, th= is task > > does __set_current_state(TASK_RUNNING), another CPU does wake_up_proces= s(task) > > which does the same UNINTERRUPTIBLE -> RUNNING transition. > > > > Looks like, this is the "data race" according to kcsan? > > Yes, they are "data races". They are probably not "race conditions" thoug= h. > > This is a fair distinction to make, and we never claimed to find "race > conditions" only I see, thanks, just wanted to be sure... > KCSAN's goal is to find *data races* according to the LKMM. Some data > races are race conditions (usually the more interesting bugs) -- but > not *all* data races are race conditions. Those are what are usually > referred to as "benign", but they can still become bugs on the wrong > arch/compiler combination. Hence, the need to annotate these accesses > with READ_ONCE, WRITE_ONCE or use atomic_t: Well, if I see READ_ONCE() in the code I want to understand why it was used. Is it really needed for correctness or we want to shut up kcsan? Say, why should wait_event(wq, *ptr) use READ_ONCE()? Nevermind, please forget. Btw, why __kcsan_check_watchpoint() does user_access_save() before try_consume_watchpoint() ? Oleg. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-2.mimecast.com ([207.211.31.81]:42090 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2404804AbfJWQYr (ORCPT ); Wed, 23 Oct 2019 12:24:47 -0400 Date: Wed, 23 Oct 2019 18:24:32 +0200 From: Oleg Nesterov Subject: Re: [PATCH v2 1/8] kcsan: Add Kernel Concurrency Sanitizer infrastructure Message-ID: <20191023162432.GC14327@redhat.com> References: <20191017141305.146193-1-elver@google.com> <20191017141305.146193-2-elver@google.com> <20191022154858.GA13700@redhat.com> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline 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 , 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" , Peter Zijlstra , 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: <20191023162432.YlH3xXwvxI6cxTVv8a11mkgiBodlXQ6Wo5L_da1np38@z> On 10/22, Marco Elver wrote: > > On Tue, 22 Oct 2019 at 17:49, Oleg Nesterov wrote: > > > > Just for example. Suppose that task->state =3D TASK_UNINTERRUPTIBLE, th= is task > > does __set_current_state(TASK_RUNNING), another CPU does wake_up_proces= s(task) > > which does the same UNINTERRUPTIBLE -> RUNNING transition. > > > > Looks like, this is the "data race" according to kcsan? > > Yes, they are "data races". They are probably not "race conditions" thoug= h. > > This is a fair distinction to make, and we never claimed to find "race > conditions" only I see, thanks, just wanted to be sure... > KCSAN's goal is to find *data races* according to the LKMM. Some data > races are race conditions (usually the more interesting bugs) -- but > not *all* data races are race conditions. Those are what are usually > referred to as "benign", but they can still become bugs on the wrong > arch/compiler combination. Hence, the need to annotate these accesses > with READ_ONCE, WRITE_ONCE or use atomic_t: Well, if I see READ_ONCE() in the code I want to understand why it was used. Is it really needed for correctness or we want to shut up kcsan? Say, why should wait_event(wq, *ptr) use READ_ONCE()? Nevermind, please forget. Btw, why __kcsan_check_watchpoint() does user_access_save() before try_consume_watchpoint() ? Oleg.