Hi,

On 2022/11/21 14:28, Christophe Leroy wrote:

Le 19/11/2022 à 15:44, Yang Yingliang a écrit :
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 <[email protected]>
---
   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;
Why introduce that 'need_put' boolean ? Why not just use
PCI_FUNC(dev->devfn) != 0 as you do in other places ?
The 'dev' is reassigned by get_function_0(), it is not the origin one.

Thanks,
Yang

        }
        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)
The != 0 is useless, just do:

        if (PCI_FUNC(dev->devfn))

+               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)
Same

+               pci_dev_put(dev0);
        return 0;
   }

Reply via email to