Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov]

Hi Alexey

07.10.2017, 20:59, "Alexey Khoroshilov" <khoroshilov@xxxxxxxxx>:
> Is it possible to switch to a nested variant:
> mutex_lock-atomic_inc-atomic_dec-mutex_unlock
> or
> atomic_inc-mutex_lock-mutex_unlock-atomic_dec
> ?

Yeah, you are right, it is a bit messy - we drop the lock while sleeping waiting for the bus master to complete operation,
and during this period family driver has to be referenced.

But we can easily grab the reference earlier and then try to lock the bus, so the second variant will work.

> --
> Alexey
>
> On 01.10.2017 08:55, Evgeniy Polyakov wrote:
>> ÂHi Alex
>>
>> Â29.09.2017, 23:23, "Alexey Khoroshilov" <khoroshilov@xxxxxxxxx>:
>>> Âw1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
>>> Âon error paths, while they did not increment it yet.
>>>
>>> Âread_therm() unlocks bus mutex on some error paths,
>>> Âwhile it is not acquired.
>>>
>>> ÂThe patch makes sure all the functions keep the balance in usage of
>>> Âthe mutex and the THERM_REFCNT.
>>>
>>> ÂFound by Linux Driver Verification project (linuxtesting.org).
>>
>> ÂYes, this looks like a bug, thanks for finding it!
>>
>> ÂPlease update your patch to use single exit point and not a mix of returns in the body of the function.
>>
>>> ÂÂÂÂÂÂÂÂÂÂret = mutex_lock_interruptible(&dev->bus_mutex);
>>> ÂÂÂÂÂÂÂÂÂÂif (ret != 0)
>>> Â- goto post_unlock;
>>> Â+ return ret;
>>>
>>> ÂÂÂÂÂÂÂÂÂÂif (!sl->family_data) {
>>> Â- ret = -ENODEV;
>>> Â- goto pre_unlock;
>>> Â+ mutex_unlock(&dev->bus_mutex);
>>> Â+ return -ENODEV;
>>> ÂÂÂÂÂÂÂÂÂÂ}