On Sat, 2022-11-19 at 22:44 +0800, Yang Yingliang wrote: > As comment of pci_get_domain_bus_and_slot() says, it returns > a pci device with refcount increment, so when finish using it, > pci_dev_put() needs be called. > > In get_dvsec_vendor0(), in normal path, the returned pci device > is passed to dev0, so after using dev0 in the callers, it need > be put, in error path, pci_dev_put() also needs be called. > > pci_get_domain_bus_and_slot() is called when PCI_FUNC() returns > non-zero, check this before put. > > Fixes: 87db7579ebd5 ("ocxl: control via sysfs whether the FPGA is > reloaded on a link reset") > Signed-off-by: Yang Yingliang <yangyingli...@huawei.com>
It might be neater to take an additional reference on dev in get_dvsec_vendor0() in the case where dev is function 0, which would mean you could call pci_dev_put() unconditionally in the callers? Either way - I think there needs to be a comment above get_dvsec_vendor0() documenting when an additional reference needs to be released. > --- > drivers/misc/ocxl/config.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c > index e401a51596b9..4da5a2b8514c 100644 > --- a/drivers/misc/ocxl/config.c > +++ b/drivers/misc/ocxl/config.c > @@ -196,16 +196,21 @@ static int read_dvsec_vendor(struct pci_dev > *dev) > static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev > **dev0, > int *out_pos) > { > + bool need_put; > int pos; > > if (PCI_FUNC(dev->devfn) != 0) { > dev = get_function_0(dev); > if (!dev) > return -1; > + need_put = true; > } > pos = find_dvsec(dev, OCXL_DVSEC_VENDOR_ID); > - if (!pos) > + if (!pos) { > + if (need_put) > + pci_dev_put(dev); > return -1; > + } > *dev0 = dev; > *out_pos = pos; > return 0; > @@ -222,6 +227,8 @@ int ocxl_config_get_reset_reload(struct pci_dev > *dev, int *val) > > pci_read_config_dword(dev0, pos + > OCXL_DVSEC_VENDOR_RESET_RELOAD, > &reset_reload); > + if (PCI_FUNC(dev->devfn) != 0) > + pci_dev_put(dev0); > *val = !!(reset_reload & BIT(0)); > return 0; > } > @@ -243,6 +250,8 @@ int ocxl_config_set_reset_reload(struct pci_dev > *dev, int val) > reset_reload &= ~BIT(0); > pci_write_config_dword(dev0, pos + > OCXL_DVSEC_VENDOR_RESET_RELOAD, > reset_reload); > + if (PCI_FUNC(dev->devfn) != 0) > + pci_dev_put(dev0); > return 0; > } > -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited