Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755113AbcC3XZ7 (ORCPT ); Wed, 30 Mar 2016 19:25:59 -0400 Received: from lists.s-osg.org ([54.187.51.154]:59372 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753469AbcC3XZ5 (ORCPT ); Wed, 30 Mar 2016 19:25:57 -0400 Subject: Re: [PATCH] regulator: Fix deadlock during regulator registration To: Jon Hunter , Liam Girdwood , Mark Brown References: <1459354153-6352-1-git-send-email-jonathanh@nvidia.com> From: Javier Martinez Canillas Cc: linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org Message-ID: <56FC607D.3020108@osg.samsung.com> Date: Wed, 30 Mar 2016 19:25:49 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1459354153-6352-1-git-send-email-jonathanh@nvidia.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1598 Lines: 54 Hello Jon, On 03/30/2016 12:09 PM, Jon Hunter wrote: > Commit 5e3ca2b349b1 ("regulator: Try to resolve regulators supplies on > registration") added a call to regulator_resolve_supply() within > regulator_register() where the regulator_list_mutex is held. This causes > the following deadlock to occur on the Tegra114 Dalmore board when the > palmas PMIC is registered because regulator_register_resolve_supply() > calls regulator_dev_lookup() which may try to acquire the > regulator_list_mutex again. > Sorry for missing that. I didn't notice because on my machine the regulators are looked up using OF and in that case the regulator_list_mutex isn't grabbed. I believe your patch is correct, I have just one trivial comment below: > > @@ -4016,15 +4015,16 @@ scrub: > regulator_ena_gpio_free(rdev); > device_unregister(&rdev->dev); > /* device core frees rdev */ > - rdev = ERR_PTR(ret); > goto out; > > wash: > regulator_ena_gpio_free(rdev); > clean: > kfree(rdev); > - rdev = ERR_PTR(ret); You are doing some cleanup of the clean and scrub error paths by removing rdev and returning ERR_PTR(ret) directly. I believe that should be in a separate patch since is not related to the fix. > - goto out; > +out: > + mutex_unlock(®ulator_list_mutex); > + kfree(config); > + return ERR_PTR(ret); > } > EXPORT_SYMBOL_GPL(regulator_register); > > If you split the cleanup and address Mark's comments, feel free to add: Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America