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

[Lukas Wunner <lukas@xxxxxxxxx>]

On Sat, Jul 11, 2026 at 05:21:07PM +0200, Maurice Hieronymus wrote:
> `broken_parity_status` shares a C bitfield word in `struct pci_dev`
> with many other bits. `broken_parity_status_store()` writes it from
> sysfs at any time without taking any lock, so userspace can make it
> race with every other writer of the same word, e.g. `pci_set_master()`
> from a runtime PM resume path, and updates of neighboring bits can be
> lost.

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.

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.

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?

Perhaps a better approach is to enclose dev_attr_broken_parity_status.attr
in "#ifdef CONFIG_EDAC" so that the attribute isn't shown unless it's used.

We shouldn't have used a sysfs attribute for this in the first place
but rather a quirk.  Unfortunately 6b09ff9d7879 does not betray for which
device this was needed, so it's difficult to convert it to a quirk now.

Bjorn introduced a pci_disable_parity() API in 2021 which is used in a
quirk for certain Mellanox products:

https://lore.kernel.org/all/20210330174318.1289680-1-helgaas@xxxxxxxxxx/

Perhaps we can deprecate the sysfs attribute in favor of using quirks
for broken devices?

Thanks,

Lukas