| Message ID | 20210510095045.3299382-29-andy.shevchenko@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | leds: cleanups and fwnode refcounting bug fixes | expand |
Hi! > fwnode_get_next_child_node() bumps a reference counting of a returned variable. > We have to balance it whenever we return to the caller. This (and similar) -- in half of the drivers we hold the handle from successful probe. Is it a problem and why is it problem only for some drivers? Thanks for series, btw, I pushed out current version of the tree. Best regards, Pavel -- http://www.livejournal.com/~pavelmachek
On Fri, May 28, 2021 at 12:14:54PM +0200, Pavel Machek wrote: > Hi! > > > fwnode_get_next_child_node() bumps a reference counting of a returned variable. > > We have to balance it whenever we return to the caller. > > This (and similar) -- in half of the drivers we hold the handle from > successful probe. Is it a problem and why is it problem only for some > drivers? Hmm... I'm not sure I have understood the question correctly. Any examples of the driver that you think needs some attention? In general the idea is that these kind of for-loops or getting next fwnode should be balanced. In case of for-loops the error or any other breakage means that reference count is bumped, for the get_next API it's always the case. I have checked between drivers and only considered above cases. Wherever there is a for-loop which isn't broken, we are fine. Wherever we have explicit reference counter drop for get_next cases, we are fine. If (any) framework requires the resource to be present that framework should bump and drop reference count on the resource by itself (so I split LED framework out from the consideration and consider that it does the right things) > Thanks for series, btw, I pushed out current version of the tree. Should I rebase the new version on something I can find in your Git tree? -- With Best Regards, Andy Shevchenko
On Fri, May 28, 2021 at 2:01 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Fri, May 28, 2021 at 12:14:54PM +0200, Pavel Machek wrote: > > Hi! > > > > > fwnode_get_next_child_node() bumps a reference counting of a returned variable. > > > We have to balance it whenever we return to the caller. > > > > This (and similar) -- in half of the drivers we hold the handle from > > successful probe. Is it a problem and why is it problem only for some > > drivers? > > Hmm... I'm not sure I have understood the question correctly. Any examples of > the driver that you think needs some attention? > > In general the idea is that these kind of for-loops or getting next fwnode > should be balanced. > > In case of for-loops the error or any other breakage means that reference count > is bumped, for the get_next API it's always the case. > > I have checked between drivers and only considered above cases. Wherever there > is a for-loop which isn't broken, we are fine. Wherever we have explicit > reference counter drop for get_next cases, we are fine. If (any) framework > requires the resource to be present that framework should bump and drop > reference count on the resource by itself (so I split LED framework out from > the consideration and consider that it does the right things) > > > Thanks for series, btw, I pushed out current version of the tree. > > Should I rebase the new version on something I can find in your Git tree? I found the above is good justification, so I leave those patches unchanged in v2. -- With Best Regards, Andy Shevchenko
diff --git a/drivers/leds/leds-sgm3140.c b/drivers/leds/leds-sgm3140.c index f4f831570f11..df9402071695 100644 --- a/drivers/leds/leds-sgm3140.c +++ b/drivers/leds/leds-sgm3140.c @@ -266,12 +266,8 @@ static int sgm3140_probe(struct platform_device *pdev) child_node, fled_cdev, NULL, &v4l2_sd_cfg); - if (IS_ERR(priv->v4l2_flash)) { - ret = PTR_ERR(priv->v4l2_flash); - goto err; - } - - return ret; + fwnode_handle_put(child_node); + return PTR_ERR_OR_ZERO(priv->v4l2_flash); err: fwnode_handle_put(child_node);
fwnode_get_next_child_node() bumps a reference counting of a returned variable. We have to balance it whenever we return to the caller. Fixes: cef8ec8cbd21 ("leds: add sgm3140 driver") Cc: Luca Weiss <luca@z3ntu.xyz> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- drivers/leds/leds-sgm3140.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)