Remove unnecessary module reference counting from the core components of the subsystem. Low-level driver modules cannot be removed before core modules since they use their exported symbols. For more context, refer to this thread: https://lore.kernel.org/linux-fpga/ZS6hhlvjUcqyv8zL@yilunxu-OptiPlex-7050 Other changes: In __fpga_bridge_get(): do a (missing ?) get_device() and bind the image to the bridge only after the mutex has been acquired. In __fpga_mgr_get(): do a get_device(). Currently, get_device() is called when allocating an image in fpga_image_info_alloc(). However, since there are still two (of_)fpga_mgr_get() functions exposed by the core, I think they should behave as expected. In fpga_region_get() / fpga_region_put(): call get_device() before acquiring the mutex and put_device() after having released the mutex to avoid races. Fixes: 654ba4cc0f3e ("fpga manager: ensure lifetime with of_fpga_mgr_get") Signed-off-by: Marco Pagani <marpagan@xxxxxxxxxx> --- drivers/fpga/fpga-bridge.c | 24 +++++++----------------- drivers/fpga/fpga-mgr.c | 8 +------- drivers/fpga/fpga-region.c | 14 ++++---------- 3 files changed, 12 insertions(+), 34 deletions(-) diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c index a024be2b84e2..3bcc9c9849c5 100644 --- a/drivers/fpga/fpga-bridge.c +++ b/drivers/fpga/fpga-bridge.c @@ -58,30 +58,21 @@ EXPORT_SYMBOL_GPL(fpga_bridge_disable); static struct fpga_bridge *__fpga_bridge_get(struct device *dev, struct fpga_image_info *info) { - struct fpga_bridge *bridge; - int ret = -ENODEV; - - bridge = to_fpga_bridge(dev); + struct fpga_bridge *bridge = to_fpga_bridge(dev); - bridge->info = info; + get_device(dev); if (!mutex_trylock(&bridge->mutex)) { - ret = -EBUSY; - goto err_dev; + dev_dbg(dev, "%s: FPGA Bridge already in use\n", __func__); + put_device(dev); + return ERR_PTR(-EBUSY); } - if (!try_module_get(dev->parent->driver->owner)) - goto err_ll_mod; + bridge->info = info; dev_dbg(&bridge->dev, "get\n"); return bridge; - -err_ll_mod: - mutex_unlock(&bridge->mutex); -err_dev: - put_device(dev); - return ERR_PTR(ret); } /** @@ -93,7 +84,7 @@ static struct fpga_bridge *__fpga_bridge_get(struct device *dev, * Return: * * fpga_bridge struct pointer if successful. * * -EBUSY if someone already has a reference to the bridge. - * * -ENODEV if @np is not an FPGA Bridge or can't take parent driver refcount. + * * -ENODEV if @np is not an FPGA Bridge. */ struct fpga_bridge *of_fpga_bridge_get(struct device_node *np, struct fpga_image_info *info) @@ -146,7 +137,6 @@ void fpga_bridge_put(struct fpga_bridge *bridge) dev_dbg(&bridge->dev, "put\n"); bridge->info = NULL; - module_put(bridge->dev.parent->driver->owner); mutex_unlock(&bridge->mutex); put_device(&bridge->dev); } diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index 06651389c592..6c355eafd18f 100644 --- a/drivers/fpga/fpga-mgr.c +++ b/drivers/fpga/fpga-mgr.c @@ -670,14 +670,9 @@ static struct fpga_manager *__fpga_mgr_get(struct device *dev) mgr = to_fpga_manager(dev); - if (!try_module_get(dev->parent->driver->owner)) - goto err_dev; + get_device(&mgr->dev); return mgr; - -err_dev: - put_device(dev); - return ERR_PTR(-ENODEV); } static int fpga_mgr_dev_match(struct device *dev, const void *data) @@ -727,7 +722,6 @@ EXPORT_SYMBOL_GPL(of_fpga_mgr_get); */ void fpga_mgr_put(struct fpga_manager *mgr) { - module_put(mgr->dev.parent->driver->owner); put_device(&mgr->dev); } EXPORT_SYMBOL_GPL(fpga_mgr_put); diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index b364a929425c..c299956cafdc 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -41,22 +41,17 @@ EXPORT_SYMBOL_GPL(fpga_region_class_find); * Return: * * fpga_region struct if successful. * * -EBUSY if someone already has a reference to the region. - * * -ENODEV if can't take parent driver module refcount. */ static struct fpga_region *fpga_region_get(struct fpga_region *region) { struct device *dev = ®ion->dev; + get_device(dev); + if (!mutex_trylock(®ion->mutex)) { dev_dbg(dev, "%s: FPGA Region already in use\n", __func__); - return ERR_PTR(-EBUSY); - } - - get_device(dev); - if (!try_module_get(dev->parent->driver->owner)) { put_device(dev); - mutex_unlock(®ion->mutex); - return ERR_PTR(-ENODEV); + return ERR_PTR(-EBUSY); } dev_dbg(dev, "get\n"); @@ -75,9 +70,8 @@ static void fpga_region_put(struct fpga_region *region) dev_dbg(dev, "put\n"); - module_put(dev->parent->driver->owner); - put_device(dev); mutex_unlock(®ion->mutex); + put_device(dev); } /** -- 2.41.0