On Thu, 2020-07-30 at 11:09 -0500, Bjorn Helgaas wrote:
> I think it would be better to have a warning once per device, so if
> XYZ device has a problem and we look at the dmesg log, we would find a
> single message for device XYZ as a hint. Would that reduce the
> nuisance level enough?
We would be OK with that.
> So I think I did it wrong in fb2659230120 ("PCI: Warn on possible RW1C
> corruption for sub-32 bit config writes"). Ratelimiting is the wrong
> concept because what we want is a single warning per device, not a
> limit on the similar messages for *all* devices, maybe something like
> this:
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index 79c4a2ef269a..e5f956b7e3b7 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -160,9 +160,12 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn,
> * write happen to have any RW1C (write-one-to-clear) bits set, we
> * just inadvertently cleared something we shouldn't have.
> */
> - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n",
> + if (!(bus->unsafe_warn & (1 << devfn))) {
> + dev_warn(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n",
> size, pci_domain_nr(bus), bus->number,
> PCI_SLOT(devfn), PCI_FUNC(devfn), where);
> + bus->unsafe_warn |= 1 << devfn;
> + }
As I understand it, devfn is an 8-bit value with five bits of device
and three bits of function. So (1 << devfn) is not going to fit in an
8-bit mask. Am I missing something here? (I do admit that my PCI
knowledge is not great).
