From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755286AbdJITNN (ORCPT ); Mon, 9 Oct 2017 15:13:13 -0400 Received: from forward3o.cmail.yandex.net ([37.9.109.247]:57527 "EHLO forward3o.cmail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754210AbdJITNM (ORCPT ); Mon, 9 Oct 2017 15:13:12 -0400 Authentication-Results: mxback11j.mail.yandex.net; dkim=pass header.i=@yandex.ru From: Evgeniy Polyakov Envelope-From: drustafa@yandex.ru To: Alexey Khoroshilov , Greg Kroah-Hartman Cc: "linux-kernel@vger.kernel.org" , "ldv-project@linuxtesting.org" In-Reply-To: References: <1506716604-23455-1-git-send-email-khoroshilov@ispras.ru> <152531506837309@web6g.yandex.ru> Subject: Re: [PATCH] w1: keep balance of mutex locks and refcnts MIME-Version: 1.0 Message-Id: <619081507576389@web21j.yandex.ru> X-Mailer: Yamail [ http://yandex.ru ] 5.0 Date: Mon, 09 Oct 2017 22:13:09 +0300 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=utf-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Alexey 07.10.2017, 20:59, "Alexey Khoroshilov" : > 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" : >>>  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; >>>           }