From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8F550C43331 for ; Wed, 13 Nov 2019 15:00:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 59C0A222D0 for ; Wed, 13 Nov 2019 15:00:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="HBHsCShK" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727922AbfKMPAh (ORCPT ); Wed, 13 Nov 2019 10:00:37 -0500 Received: from mail-ot1-f66.google.com ([209.85.210.66]:46594 "EHLO mail-ot1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727550AbfKMPAg (ORCPT ); Wed, 13 Nov 2019 10:00:36 -0500 Received: by mail-ot1-f66.google.com with SMTP id n23so1819543otr.13 for ; Wed, 13 Nov 2019 07:00:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hn0rE8EvQLayCzvR+VM/mjAxPaLh9HP6Et6bUGhBo8g=; b=HBHsCShKPxflrvZ7COZyam7cSyPMVSE2Df2KInwngnmj3XC6CG1iIdad+zrmgxjpoL pSxx4/TtFbnMa8p0ZzauIadXVwYEW8KF0fCHGOYcggwDmdl//Oog+V52wV3Flug/P1xp tGL4mvdwDE/9Y8unvP2kivCJXLeetnnoiVjsYJwtZwlp7+cp3seRr7udcMQLGDVX8Xus p3tPTd4KIL68djgdYqFrWe1NHLSxqA25dDRo8Ax/Ah0RoEO4FcK3d6EL95e6TVEyJJ1V QnX0eZljqiSf+2Rb+ggZUNKxx58eTL0jxZ4uByrkKDFwEZe6fUdBiGg6md11I/g/g5Cx FCaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hn0rE8EvQLayCzvR+VM/mjAxPaLh9HP6Et6bUGhBo8g=; b=oy+6qibxdQA0mTzrQN2olczm9Q3LgDIswv3yNwbEfOnS+5yrOH4vcG8FGvIPUWsmQR NxJIXbKT0ARyyNiFTZgl50LDyPzLdF/zgXEcyDHVwSLWFCpFD7FmN9YeZ/uXN9SH7GKw suEUBYK4J71XEu6S6UxvjkmbnyD8XIbfSzxMtIxbCHKKIZLeizOmIiLZCL6m0oB4W/8I W+SLCDbLMnqF3NZmEDoDBK/is10jULp6ZTcHso7o0da1E4jxkYHARhVfbsMiQ3uENfGh zOubOZcK7g2xIz624+ZQirp3EL3cFqBESNAnfejOSQEkz/FdmFxzHrtjssqRcmyF00QB 6IHA== X-Gm-Message-State: APjAAAUgIIDzj2jsVhyAfblp8oE+Dry4AqCnDqnJhMBQt7KY18g/GM0b TBvXqm2jfN1HehsPaUph+lXVMJuQ2C1rgU1l4a6Z4A== X-Google-Smtp-Source: APXvYqxJ3c6MEZ0/2hGP2/5UKDNPX6bPtZFJ9h4viwgj5WeAEavitL5ck3Xe6xnzWLwZazm4TAy56EBFRkAq0wZABQk= X-Received: by 2002:a9d:69cf:: with SMTP id v15mr3119489oto.251.1573657235271; Wed, 13 Nov 2019 07:00:35 -0800 (PST) MIME-Version: 1.0 References: <20191112224441.2kxmt727qy4l4ncb@ast-mbp.dhcp.thefacebook.com> In-Reply-To: From: Marco Elver Date: Wed, 13 Nov 2019 16:00:21 +0100 Message-ID: Subject: Re: KCSAN: data-race in __alloc_file / __alloc_file To: Linus Torvalds Cc: Eric Dumazet , Alexei Starovoitov , Alan Stern , Eric Dumazet , syzbot , linux-fsdevel , Linux Kernel Mailing List , syzkaller-bugs , Al Viro , Andrea Parri , "Paul E. McKenney" , LKMM Maintainers -- Akira Yokosawa Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed, 13 Nov 2019 at 00:41, Linus Torvalds wrote: > > On Tue, Nov 12, 2019 at 3:18 PM Eric Dumazet wrote: > > > > Hmm we have the ' volatile' attribute on jiffies, and it causes > > confusion already :p > > The jiffies case is partly historical, but partly also because it's > one of the very very few data structures where 99% of all uses are > unlocked and for convenience reasons we really don't want to force > those legacy cases to do anything special about it. > > "jiffies" really is very special for those kinds of legacy reasons. > Look at the kinds of games we play with it on 32-bit architectures: > the "real" storage is "jiffies_64", but nobody actually wants to use > that, and because of load tearing issues it's actually hard to use > too. So what everybody _uses_ is just the low 32 bits, and 'jiffies' > isn't a real variable, it's done with linker tricks in our > vmlinux.lds.S files. So, for example, on sparc32, you find this: > > jiffies = jiffies_64 + 4; > > in the vmlinux.lds.S file, because it's big-endian, and the lower 32 > bits are at offset 4 from the real 64-bit variable. > > Note that to actually read the "true" full 64-bit value, you have to > then call a function that does the proper sequence counter stuff etc. > But nobody really wants it, since what everybody actually _uses_ is > the "time_after(x,jiffies+10)" kind of thing, which is only done in > the wrapping "unsigned long" time base. So the odd "linker tricks with > the atomic low bits marked as a volatile data structure" is actually > exactly what we want, but people should realize that this is not > normal. > > So 'jiffies' is really really special. > > And absolutely nothing else should use 'volatile' on data structures > and be that special. In the case of jiffies, the rule ends up being > that nobody should ever write to it (you write to the real > jiffies_64), and the real jiffies_64 thing gets the real locking, and > 'jiffies' is a read-only volatile thing. > > So "READ_ONCE()" is indeed unnecessary with jiffies, but it won't > hurt. It's not really "confusion" - there's nothing _wrong_ with using > READ_ONCE() on volatile data, but we just normally don't do volatile > data in the kernel (because our normal model is that data is never > really volatile in general - there may be locked and unlocked accesses > to it, so it's stable or volatile depending on context, and thus the > 'volatile' goes on the _code_, not on the data structure) > > But while jiffies READ_ONCE() accesses isn't _wrong_, it is also not > really paired with any WRITE_ONCE(), because the real update is to > technically not even to the same full data structure. > > So don't look to jiffies for how to do things. It's an odd one-off. > > That said, for "this might be used racily", if there are annotations > for clang to just make it shut up about one particular field in a > structure, than I think that would be ok. As long as it doesn't then > imply special code generation (outside of the checking, of course). There are annotations that work for globals only. This limitation is because once you take a pointer of a variable, and just pass around the address, the attribute will be lost. So sometimes you might do the right thing, but other times you might not. Just to summarize the options we've had so far: 1. Add a comment, and let the tool parse it somehow. 2. Add attribute to variables. 3. Add some new macro to use with expressions, which doesn't do anything if the tool is disabled. E.g. "racy__(counter++)", "lossy__(counter++);" or any suitable name. 1 and 3 are almost equivalent, except that 3 introduces "structured" comments that are much easier to understand for tooling. Both 1 and 2 need compiler support. Right now both gcc and clang support KCSAN, and doing 1 or 2 would probably imply dropping one of them. Both 1 and 2 are also brittle: 1 because comments are not forced to be structured, and 2 because attributes on variables do not propagate through pointers. >From our perspective, macro-based annotations would be most reliable. Thanks, -- Marco