Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAHk-=wjGd0Ce2xadkiErPWxVBT2mhyeZ4TKyih2sJwyE3ohdHw@mail.gmail.com>
 <Pine.LNX.4.44L0.1911121515400.1567-100000@iolanthe.rowland.org> <CAHk-=wgnjMEvqHnu_iJcbr_kdFyBQLhYojwv5T7p9F+CHxA9pg@mail.gmail.com>
In-Reply-To: <CAHk-=wgnjMEvqHnu_iJcbr_kdFyBQLhYojwv5T7p9F+CHxA9pg@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue, 12 Nov 2019 13:13:35 -0800
Message-ID: <CAHk-=wg9yCKVdenys6FfcpAc9KSxVMOTT2DddVcj-Ofu1MYNhQ@mail.gmail.com>
Subject: Re: KCSAN: data-race in __alloc_file / __alloc_file
To:     Alan Stern <stern@rowland.harvard.edu>
Cc:     Marco Elver <elver@google.com>, Eric Dumazet <edumazet@google.com>,
        Eric Dumazet <eric.dumazet@gmail.com>,
        syzbot <syzbot+3ef049d50587836c0606@syzkaller.appspotmail.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Andrea Parri <parri.andrea@gmail.com>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        LKMM Maintainers -- Akira Yokosawa <akiyks@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Nov 12, 2019 at 12:58 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Honestly, my preferred model would have been to just add a comment,
> and have the reporting tool know to then just ignore it. So something
> like
>
> +               // Benign data-race on min_flt
>                 tsk->min_flt++;
>                 perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, 1, regs, address);
>
> for the case that Eric mentioned - the tool would trigger on
> "data-race", and the rest of the comment could/should be for humans.
> Without making the code uglier, but giving the potential for a nice
> leghibl.e explanation instead of a completely illegible "let's
> randomly use WRITE_ONCE() here" or something like that.

Hmm. Looking at the practicality of this, it actually doesn't look
*too* horrible.

I note that at least clang already has a "--blacklist" ability. I
didn't find a list of complete syntax for that, and it looks like it
might be just "whole functions" or "whole source files", but maybe the
clang people would be willing to add "file and line ranges" to the
blacklists?

Then you could generate the blacklist with that trivial grep before
you start the build, and -fsanitize=thread would automatically simply
not look at those lines.

For a simple first case, maybe the rule could be that the comment has
to be on the line. A bit less legible for humans, but it could be

-               tsk->min_flt++;
+               // Benign race min_flt - statistics only
+               tsk->min_flt++; // data-race

instead.

Wouldn't that be a much better annotation than having to add code?

               Linus