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).