Re: [PATCH 2/2] PCI: Replace pci_dev->broken_parity_status with accessors

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/[email protected]/

Best,

Maurice