| Message ID | 20250321015806.954866-5-pandoh@google.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Rate limit AER logs | expand |
On 21/03/2025 02:58, Jon Pan-Doh wrote: > Update function/param names to be more descriptive. This is a > preparatory patch for when source devices are iterated through > to institue rate limits. This commit description doesn't provide enough information on why and how we make the names more descriptive. With this change, we want to make it clear that this function logs information about the Root Port that received an error message, not just a generic PCIe Port. Also, there's a typo in the subject: s/aer_printrp_info()/aer_print_rp_info()/ I'm fine with the code changes but I'd like to see the new commit description before giving the r-b. All the best, Karolina > > Signed-off-by: Jon Pan-Doh <pandoh@google.com> > Reported-by: Sargun Dhillon <sargun@meta.com> > --- > drivers/pci/pcie/aer.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c > index e5db1fdd8421..3c63a6963608 100644 > --- a/drivers/pci/pcie/aer.c > +++ b/drivers/pci/pcie/aer.c > @@ -727,15 +727,15 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info, > info->severity, info->tlp_header_valid, &info->tlp); > } > > -static void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info) > +static void aer_print_rp_info(struct pci_dev *rp, struct aer_err_info *info) > { > u8 bus = info->id >> 8; > u8 devfn = info->id & 0xff; > > - pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d\n", > + pci_info(rp, "%s%s error message received from %04x:%02x:%02x.%d\n", > info->multi_error_valid ? "Multiple " : "", > aer_error_severity_string[info->severity], > - pci_domain_nr(dev->bus), bus, PCI_SLOT(devfn), > + pci_domain_nr(rp->bus), bus, PCI_SLOT(devfn), > PCI_FUNC(devfn)); > } > > @@ -1299,7 +1299,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc, > e_info.multi_error_valid = 1; > else > e_info.multi_error_valid = 0; > - aer_print_port_info(pdev, &e_info); > + aer_print_rp_info(pdev, &e_info); > > if (find_source_device(pdev, &e_info)) > aer_process_err_devices(&e_info, KERN_WARNING); > @@ -1318,7 +1318,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc, > else > e_info.multi_error_valid = 0; > > - aer_print_port_info(pdev, &e_info); > + aer_print_rp_info(pdev, &e_info); > > if (find_source_device(pdev, &e_info)) > aer_process_err_devices(&e_info, KERN_ERR);
On Fri, Mar 21, 2025 at 6:39 AM Karolina Stolarek <karolina.stolarek@oracle.com> wrote: > This commit description doesn't provide enough information on why and > how we make the names more descriptive. With this change, we want to > make it clear that this function logs information about the Root Port > that received an error message, not just a generic PCIe Port. > > Also, there's a typo in the subject: > s/aer_printrp_info()/aer_print_rp_info()/ > > I'm fine with the code changes but I'd like to see the new commit > description before giving the r-b. Ack. Will add more detail/fix typos (incl. s/institue/institute) in v6. Something like: PCI/AER: Rename aer_print_port_info() to aer_print_rp_info() Update function/param names to be more descriptive. Make it clear that this function logs error information from a root port vs. generic PCIe port. This is a preparatory patch for instituting rate limits on logs. It frees up the dev variable for source device iteration. Thanks, Jon
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index e5db1fdd8421..3c63a6963608 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -727,15 +727,15 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info, info->severity, info->tlp_header_valid, &info->tlp); } -static void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info) +static void aer_print_rp_info(struct pci_dev *rp, struct aer_err_info *info) { u8 bus = info->id >> 8; u8 devfn = info->id & 0xff; - pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d\n", + pci_info(rp, "%s%s error message received from %04x:%02x:%02x.%d\n", info->multi_error_valid ? "Multiple " : "", aer_error_severity_string[info->severity], - pci_domain_nr(dev->bus), bus, PCI_SLOT(devfn), + pci_domain_nr(rp->bus), bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); } @@ -1299,7 +1299,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc, e_info.multi_error_valid = 1; else e_info.multi_error_valid = 0; - aer_print_port_info(pdev, &e_info); + aer_print_rp_info(pdev, &e_info); if (find_source_device(pdev, &e_info)) aer_process_err_devices(&e_info, KERN_WARNING); @@ -1318,7 +1318,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc, else e_info.multi_error_valid = 0; - aer_print_port_info(pdev, &e_info); + aer_print_rp_info(pdev, &e_info); if (find_source_device(pdev, &e_info)) aer_process_err_devices(&e_info, KERN_ERR);
Update function/param names to be more descriptive. This is a preparatory patch for when source devices are iterated through to institue rate limits. Signed-off-by: Jon Pan-Doh <pandoh@google.com> Reported-by: Sargun Dhillon <sargun@meta.com> --- drivers/pci/pcie/aer.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)