Received: by 2002:a05:6a10:6744:0:0:0:0 with SMTP id w4csp66849pxu; Tue, 6 Oct 2020 19:05:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyskkbFrD3V+4pGw0LQg8eVy4YC3dOpa/FXpJ6sgjJTcP5twCKI4jvLkJSxCbMRgHt5U7m8 X-Received: by 2002:a50:a693:: with SMTP id e19mr1115471edc.333.1602036351852; Tue, 06 Oct 2020 19:05:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1602036351; cv=none; d=google.com; s=arc-20160816; b=pi6C0vpE0A0CQP4itUWE+Q/Ha8EKkX680vaa9mBMBHM79ZgsiQYr09F7h9/y/SRxQd KZfM5S1wqhKhwV73yDQn4zQX+5CWdYoZZXk1N21b17gLDD91nhKAlJVerPC0PBsGCaAT HFBs0g998x+4ma0Egdlx0cg4Na8xSu03mHCdn3K/w7XMWelmi3hS/FFI7PEnRFhqGJ8p VtQ5kno/ymsemHndhk50wC26qQ3RgXaWOHnHXiWiZB9JdDxVghx8RFU3o2ff1h+F7LZt TGHMj0Z70yUPv3XhsKE17NBp8LWMCrI3DlhyEaVTYeN0XKWEQ9BC/NpWilsGojV4EB8L eljA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=6Q9bSmskWSEkb9gd9wms6mpxggsv5bcsjx2/k1X765I=; b=Ib7FPNWw2JCl6kqhBFEUkr3mu6MA4NTCGGIIchS7qUFIkQ7bs1IeV14EPlfbIjJCeB 547aAeAfSBteLmT9sTmKsXWib1ANDwuOsv6mulr4a7Sk4/9x4zAO5SXqDMY78FTtULFp w+rYyeuzXUFTEFYM8ypZOzzikizZHP3+r2y4fgdWaBmajWlAS6WEQzZlKdXiInRaYvJy KyzVH7j3GTC+hm8VTPItpUfkNqbnHwyAoAqAS9ZyxRVXUq5eIw8f+DO5NmI6jTdQiVfP 2Dgx5zUQOc5ugCu7b9CvSj0rSKzFBHCuy9LNpEtlpCe4wd4053gk1D4CQU/374ezg1qk LLfg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=ou4pOjTX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id lu21si411793ejb.482.2020.10.06.19.05.27; Tue, 06 Oct 2020 19:05:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=ou4pOjTX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726564AbgJGBrx (ORCPT + 99 others); Tue, 6 Oct 2020 21:47:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55556 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725947AbgJGBrw (ORCPT ); Tue, 6 Oct 2020 21:47:52 -0400 Received: from mail-io1-xd2c.google.com (mail-io1-xd2c.google.com [IPv6:2607:f8b0:4864:20::d2c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3A0A7C061755; Tue, 6 Oct 2020 18:47:52 -0700 (PDT) Received: by mail-io1-xd2c.google.com with SMTP id n6so658961ioc.12; Tue, 06 Oct 2020 18:47:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6Q9bSmskWSEkb9gd9wms6mpxggsv5bcsjx2/k1X765I=; b=ou4pOjTXlapUdIXJIJdGYGjfsLyiC/GDbq31d7UEU4ayikIQkDv8xsrzNBimSiQvRD hyj4uIqxBJbbfFUzV9mNGYK6Irh+77wP+oSkeVtxdGrZsBqBMXwmqVJKIZIOdaf7+1QJ Hy839G7LOS20O+F292EwGXIXRc2zQm+pORwba/k/bE0kCoSgTOoRmImcCd3UCtcU3ept pFNZ+OFMCwDEvNcOUIRYqg4SG2F+0RTDpWczShOlDY4KO8oQMJvCQV2VoHf07gyeh2hw P5JL5fdoFvr8ZLLR1FtP+IZTvg+Ul2z3aFcti/Y2p3Ju6fnkm74+cwTT9X1bxG1vKEoj NMiA== 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=6Q9bSmskWSEkb9gd9wms6mpxggsv5bcsjx2/k1X765I=; b=cHEg9XBW+s8eqP/yZDu6Zh/i2uUfGs6e4TgsbsIb/76ra/G+Z7SQJMkZHUVmiKE0xn CtyxmG8f4B52nM+Mt3cZBe7tgsba/JY6TvDjBX7s5U6zsleburv+muvukhCefuq+t9O/ NaIH2h5hXpwfHVMTHasEmnElHbPYCNiu+TB5TMpYzNvzLlXJBF5h0fAjLn9+smudOgGO 8qtYA/K7izws7I3bDDPVtKsME/d5syByZO5h4fqGct3ySEFKuDrbMKz0B2QKnAdeWLvS K73Pg9sPEQmaVU0opV+p9gV9dmaCfhs7drtueoxqqyeBen5SW3V0a0LWoAncKnlAQ0E5 bgdA== X-Gm-Message-State: AOAM530fSUDqebzQ3ZQlSM0G3aCEz92zzdUCVHgKcozoK29N4vpWhFZS abCOlFZqQAcJ4P/jkjn1bIpymzzNXdZCQ8R5dbE= X-Received: by 2002:a02:c914:: with SMTP id t20mr921216jao.117.1602035271378; Tue, 06 Oct 2020 18:47:51 -0700 (PDT) MIME-Version: 1.0 References: <20200909112850.hbtgkvwqy2rlixst@pali> <20201006222222.GA3221382@bjorn-Precision-5520> In-Reply-To: <20201006222222.GA3221382@bjorn-Precision-5520> From: "Oliver O'Halloran" Date: Wed, 7 Oct 2020 12:47:40 +1100 Message-ID: Subject: Re: PCI: Race condition in pci_create_sysfs_dev_files To: Bjorn Helgaas Cc: =?UTF-8?Q?Pali_Roh=C3=A1r?= , Bjorn Helgaas , linux-pci , Linux Kernel Mailing List , Lorenzo Pieralisi , Gregory Clement , Andrew Lunn , =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= , Yinghai Lu Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 7, 2020 at 10:26 AM Bjorn Helgaas wrote: > > I'm not really a fan of this because pci_sysfs_init() is a bit of a > hack to begin with, and this makes it even more complicated. > > It's not obvious from the code why we need pci_sysfs_init(), but > Yinghai hinted [1] that we need to create sysfs after assigning > resources. I experimented by removing pci_sysfs_init() and skipping > the ROM BAR sizing. In that case, we create sysfs files in > pci_bus_add_device() and later assign space for the ROM BAR, so we > fail to create the "rom" sysfs file. > > The current solution to that is to delay the sysfs files until > pci_sysfs_init(), a late_initcall(), which runs after resource > assignments. But I think it would be better if we could create the > sysfs file when we assign the BAR. Then we could get rid of the > late_initcall() and that implicit ordering requirement. You could probably fix that by using an attribute_group to control whether the attribute shows up in sysfs or not. The .is_visible() for the group can look at the current state of the device and hide the rom attribute if the BAR isn't assigned or doesn't exist. That way we don't need to care when the actual assignment occurs. > But I haven't tried to code it up, so it's probably more complicated > than this. I guess ideally we would assign all the resources before > pci_bus_add_device(). If we could do that, we could just remove > pci_sysfs_init() and everything would just work, but I think that's a > HUGE can of worms. I was under the impression the whole point of pci_bus_add_device() was to handle any initialisation that needed to be done after resources were assigned. Is the ROM BAR being potentially unassigned an x86ism or is there some bigger point I'm missing? Oliver