[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] PCI: Replace pci_dev->broken_parity_status with accessors
- To: "Lukas Wunner" <lukas@xxxxxxxxx>, "Maurice Hieronymus" <mhi@xxxxxxxxxxx>
- From: "Maurice Hieronymus" <mhi@xxxxxxxxxxx>
- Date: Tue, 14 Jul 2026 21:36:44 +0200
- Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=mail20150812 header.d=mailbox.org header.i="@mailbox.org" header.h="From:Subject:Date:Message-Id:To:Cc:Mime-Version:Content-Type:Content-Transfer-Encoding:In-Reply-To:References"; dkim=pass header.s=mail20150812 header.d=mailbox.org header.i="@mailbox.org" header.h="From:Subject:Date:Message-Id:To:Cc:Mime-Version:Content-Type:Content-Transfer-Encoding:In-Reply-To:References"
- Authentication-results: outgoing_mbo_mout; dkim=pass header.d=mailbox.org header.s=mail20150812 header.b=xvjOuN1v; spf=pass (outgoing_mbo_mout: domain of mhi@xxxxxxxxxxx designates 2001:67c:2050:b231:465::102 as permitted sender) smtp.mailfrom=mhi@xxxxxxxxxxx
- Cc: "Edward Cree" <ecree.xilinx@xxxxxxxxx>, "Andrew Lunn" <andrew+netdev@xxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, "Eric Dumazet" <edumazet@xxxxxxxxxx>, "Jakub Kicinski" <kuba@xxxxxxxxxx>, "Paolo Abeni" <pabeni@xxxxxxxxxx>, "Bjorn Helgaas" <bhelgaas@xxxxxxxxxx>, "Justin Tee" <justin.tee@xxxxxxxxxxxx>, "Paul Ely" <paul.ely@xxxxxxxxxxxx>, "James E.J. Bottomley" <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>, "Martin K. Petersen" <martin.petersen@xxxxxxxxxx>, "Juergen Gross" <jgross@xxxxxxxx>, "Stefano Stabellini" <sstabellini@xxxxxxxxxx>, "Oleksandr Tyshchenko" <oleksandr_tyshchenko@xxxxxxxx>, "Miguel Ojeda" <ojeda@xxxxxxxxxx>, "Boqun Feng" <boqun@xxxxxxxxxx>, "Gary Guo" <gary@xxxxxxxxxxx>, Björn Roy Baron <bjorn3_gh@xxxxxxxxxxxxxx>, "Benno Lossin" <lossin@xxxxxxxxxx>, "Andreas Hindborg" <a.hindborg@xxxxxxxxxx>, "Alice Ryhl" <aliceryhl@xxxxxxxxxx>, "Trevor Gross" <tmgross@xxxxxxxxx>, "Daniel Almeida" <daniel.almeida@xxxxxxxxxxxxx>, "Tamir Duberstein" <tamird@xxxxxxxxxx>, "Alexandre Courbot" <acourbot@xxxxxxxxxx>, Onur Özkan <work@xxxxxxxxxxxxx>, "Borislav Petkov" <bp@xxxxxxxxx>, "Tony Luck" <tony.luck@xxxxxxxxx>, "Danilo Krummrich" <dakr@xxxxxxxxxx>, <rust-for-linux@xxxxxxxxxxxxxxx>, <netdev@xxxxxxxxxxxxxxx>, <linux-net-drivers@xxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>, <linux-pci@xxxxxxxxxxxxxxx>, <linux-scsi@xxxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>, <linux-edac@xxxxxxxxxxxxxxx>
- Delivery-date: Tue, 14 Jul 2026 19:37:02 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Sun Jul 12, 2026 at 4:55 PM CEST, Lukas Wunner wrote:
>
> For static bits in struct pci_dev, i.e. ones that are mostly read
> and almost never written, and in particular ones that are only
> written on device enumeration, it's perfectly fine and more convenient
> to keep them as bitfields. broken_parity_status seems to fit that bill.
>
Its only writer is broken_parity_status_store(), so it is written from
sysfs at arbitrary times rather than on enumeration. But I agree the
exposure is small; apart from the sysfs attribute itself, EDAC is the
only reader.
> For other bits which are modified more frequently, move them to the existing
> priv_flags member if you believe they can be updated concurrently.
> I'm not sure is_busmaster fits that bill, it isn't updated that often.
>
The motivation is not update frequency. The planned Rust device
enabling rework [1] wants a guard object whose drop calls
pci_disable_device(), which may then run concurrently with
pci_set_master(). Moving is_busmaster out of the bitfield stops those
paths from writing to the word shared with the other bits.
> Quite honestly I'm wondering if there is anything to fix here.
> Yes I get it, userspace may interfere with adjacent bits.
> But broken_parity_status is only used for certain broken devices
> on EDAC-capable platforms. That's a fringe use case.
> Is it really worth refactoring this?
>
I dropped this patch in v2 and moved only is_busmaster, into
priv_flags as you suggested.
> Perhaps we can deprecate the sysfs attribute in favor of using quirks
> for broken devices?
>
That sounds like the better long term fix to me, but I would leave it
to a separate discussion.
[1]
https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@xxxxxxxxxx/
Best,
Maurice
|