| Message ID | 20200613215923.2611-1-wu000273@umn.edu |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: xiic: Fix reference count leaks. | expand |
Hi Qiushi, On 17. 06. 20 16:30, David Laight wrote: > From: Wolfram Sang >> Sent: 14 June 2020 10:10 >> >> On Sat, Jun 13, 2020 at 04:59:23PM -0500, wu000273@umn.edu wrote: >>> From: Qiushi Wu <wu000273@umn.edu> >>> >>> pm_runtime_get_sync() increments the runtime PM usage counter even >>> when it returns an error code. Thus call pm_runtime_put_noidle() >>> if pm_runtime_get_sync() fails. >> >> Can you point me to a discussion where it was decided that this is a >> proper fix? I'd think we rather should fix pm_runtime_get_sync() but >> maybe there are technical reasons against it. > > Or, if there is one place that actually needs the reference split the > code so that unusual case keeps the reference. > > In one of the patches I also spotted: > ret = pm_runtime_get_sync(); > if (ret < 0 && ret != _EAGAIN) > ... > > (I think it was EAGAIN.) > I can't help feeling that is just wrong somewhere. Qiushi: Any update on this one? Thanks, Michal
Hi Qiushi, On 13. 07. 20 21:41, Qiushi Wu wrote: > Hi Michal, > I think multiple previous patches also fixed similar problems, such as > https://patchwork.kernel.org/patch/2404751/ > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2227849.html > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao.liu@zju.edu.cn/ > > But recently, people are discussing reimplementing > function pm_runtime_get_sync(), because it often misguides users and > introducing bugs. > So at this time point, I'm not sure if we should fix this issue in the > current version or wait for the new implementation of this API. : ) If we apply this patch then it needs to be reverted when API is fixed. Is there any timeline mentioned? If this will take some time I am fine with applying this patch. Thanks, Michal
Hi, On 27. 07. 20 8:16, Qiushi Wu wrote: > Hi Michal, > > Thanks for your reply! > I checked the mailing list for the related emails against this API, but > I didn't find any specific timeline they mentioned. > Maybe we can patch this bug to prevent potential issues at this time > point if you are willing to. ok. I am fine with this but please send v2 and add there some comments about it and extend commit message with more information you provided. I just want to make sure that in several months timeframe when someone look at this it will be clear that it was workaround. Thanks, Michal
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 90c1c362394d..ffec41e6be72 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -696,8 +696,10 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) xiic_getreg8(i2c, XIIC_SR_REG_OFFSET)); err = pm_runtime_get_sync(i2c->dev); - if (err < 0) + if (err < 0) { + pm_runtime_put_noidle(i2c->dev); return err; + } err = xiic_busy(i2c); if (err) @@ -860,8 +862,10 @@ static int xiic_i2c_remove(struct platform_device *pdev) i2c_del_adapter(&i2c->adap); ret = pm_runtime_get_sync(i2c->dev); - if (ret < 0) + if (ret < 0) { + pm_runtime_put_noidle(i2c->dev); return ret; + } xiic_deinit(i2c); pm_runtime_put_sync(i2c->dev);