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=-0.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 954D1C2D0F4 for ; Wed, 8 Apr 2020 21:15:00 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 31AC620771 for ; Wed, 8 Apr 2020 21:15:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="ZCJNoBCZ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 31AC620771 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9094A8E000D; Wed, 8 Apr 2020 17:14:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8B9768E0006; Wed, 8 Apr 2020 17:14:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7D00F8E000D; Wed, 8 Apr 2020 17:14:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0111.hostedemail.com [216.40.44.111]) by kanga.kvack.org (Postfix) with ESMTP id 62EFD8E0006 for ; Wed, 8 Apr 2020 17:14:59 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 27F278248D7C for ; Wed, 8 Apr 2020 21:14:59 +0000 (UTC) X-FDA: 76685942718.29.offer44_4899194fc112b X-HE-Tag: offer44_4899194fc112b X-Filterd-Recvd-Size: 6654 Received: from mail-lf1-f65.google.com (mail-lf1-f65.google.com [209.85.167.65]) by imf37.hostedemail.com (Postfix) with ESMTP for ; Wed, 8 Apr 2020 21:14:58 +0000 (UTC) Received: by mail-lf1-f65.google.com with SMTP id z23so6277590lfh.8 for ; Wed, 08 Apr 2020 14:14:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=tatDUkEu74fJwPgLRw2JZ+S0O3CWrMQzKjdHk2GEH4c=; b=ZCJNoBCZFsc4OT/9i1qz85T2vqFB754V8LwcT9UtL7kperRB0ACYsy9mwUhTMOwdTZ cyQIo+ysWh7Xu2ZnUStxeozjPNVYxyF/sjojmf/jmD2/NHrfZy72cD82mFPs3G2o0Hqf LZeDKiFF50stQUT8aTCHhIfZ+qMj4bo1DHtwc= 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=tatDUkEu74fJwPgLRw2JZ+S0O3CWrMQzKjdHk2GEH4c=; b=PWcH2Rv9+78Bzyf9FowCZYq/taMZK56OXwhvy/pDIIdvkBwoxX/sneMPl/GgoL0roQ xTjTq6PrmoCVgP35lQY6HMPQCnOWsXOUatGAywPeP6UZReWeo0oMiQEOXEVkfjw9Rmw/ qPvE/yCue2ofVSTk7gns1IJCa7m8vFz99TJMDhdxT8D3dmsY9YH+bYou5FWXdGQldDnZ lwx/sX6Bua1kLmjSqTTYXI6UkcORGOUWbLPg+cG/c4YEz7MeiW9NRIH/xxuPUWQtnLYf Soud4Mdx6HyG8gCmSzLARhw6z0UQqXSXCV9+Sr2+coiWj3E4cZace0ywMS8k4gFwpEqW y+Qg== X-Gm-Message-State: AGi0PuY8wRXJ50Sy5Jq0G2r28+zVnTATzEHcQ8zFp2bktFLBnh408Puu toRGbIMCA/qcxFVxh12C6VbIdkwV6Tw= X-Google-Smtp-Source: APiQypLLSKKbSZ2ehPPxHRBBFPUzDys/pcJIReEGMP9MTbkiUHInMMZh5nRRrp4JR9id32MTAw7+EA== X-Received: by 2002:a05:6512:490:: with SMTP id v16mr1896210lfq.71.1586380496181; Wed, 08 Apr 2020 14:14:56 -0700 (PDT) Received: from mail-lj1-f174.google.com (mail-lj1-f174.google.com. [209.85.208.174]) by smtp.gmail.com with ESMTPSA id e4sm15907666lfn.80.2020.04.08.14.14.54 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 08 Apr 2020 14:14:54 -0700 (PDT) Received: by mail-lj1-f174.google.com with SMTP id k21so9286570ljh.2 for ; Wed, 08 Apr 2020 14:14:54 -0700 (PDT) X-Received: by 2002:a2e:a58e:: with SMTP id m14mr6104348ljp.204.1586380494037; Wed, 08 Apr 2020 14:14:54 -0700 (PDT) MIME-Version: 1.0 References: <20200406200254.a69ebd9e08c4074e41ddebaf@linux-foundation.org> <20200407031042.8o-fYMox-%akpm@linux-foundation.org> <20200408202630.GA1666@paulmck-ThinkPad-P72> In-Reply-To: <20200408202630.GA1666@paulmck-ThinkPad-P72> From: Linus Torvalds Date: Wed, 8 Apr 2020 14:14:38 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [patch 125/166] lib/list: prevent compiler reloads inside 'safe' list iteration To: "Paul E. McKenney" Cc: Andrew Morton , Chris Wilson , David Laight , Marco Elver , Linux-MM , Mark Rutland , mm-commits@vger.kernel.org, Randy Dunlap , stable Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Apr 8, 2020 at 1:26 PM Paul E. McKenney wrote: > > OK, it sounds like we need to specify what needs to be present in this > sort of commit log. That would have helped. But in this case, after having looked more at it, it wasn't that the commit log was not sufficient, it was that the change was wrong. With a better commit log and a pointer to the KCSAN report, I would have been able to make a better judgment call earlier, and not have had to ask for more information. But once I figured out what the problem was, it was clear that (a) what the i915 driver does is at a minimum questionable, and quite likely actively buggy (b) even if it's not buggy in the i915 driver, changing the list traversal macros to use READ_ONCE() would hide other places where it most definitely is buggy. > o The KCSAN splat, optionally including file/line numbers. I do think that the KCSAN splat - very preferably simplified and with explanations - should basically always be included if KCSAN was the reason. Of course, the "simplified and with explanations" may be simplified to the point where none of the original splat even remains. Those splats aren't so legible that anybody should feel like they should be included. Put another way: an analysis that is so thorough that it makes the raw splat data pointless is a *good* thing, and if that means that no splat is needed, all the better. > o Detailed description of the problem this splat identifies, for > example, the code might fail to acquire a necessary lock, a plain > load might vulnerable to compiler optimizations, and so on. See above: if the description is detailed enough, then the splat itself becomes much less interesting. But in this case, for example, it's worth pointing out that the "safe" list accessors are used all over the kernel, and they are very much _not_ thread-safe. The "safe" in them is purely about the current thread removing an entry, not some kind of general thread-safeness. Which is why I decided I really hated that patch - it basically would make KCSAN unable to find _real_ races elsewhere, because it hid a very special race in the i915 driver. So I started out with the "that can't be right" kind of general feeling of uneasiness about the patch. I ended up with a much more specific "no, that's very much not right" and no amount of commit log should have saved it. As mentioned, I suspect that the i915 driver could actually use the RCU-safe list walkers. Their particular use is not about RCU per se, but it has some very similar rules about walking a list concurrently with somebody adding an entry to it. And the RCU list walkers do end up doing special things to load the next pointer. Whether we want to say - and document - that "ok, the RCU list walkers are also useful for this non-RCU use case" in general might be worth discussing. It may be that the i915 use case is so special that it's only worth documenting there. Linus