From mboxrd@z Thu Jan 1 00:00:00 1970 From: Viresh Kumar Subject: Re: [PATCH 04/17] PM / OPP: Introduce dev_pm_opp_get_max_volt_latency() Date: Wed, 13 Jan 2016 11:04:47 +0530 Message-ID: <20160113053447.GB6050@ubuntu> References: <08691b482198b0709fd258d310a5e6ecda2f1a18.1450777582.git.viresh.kumar@linaro.org> <20160112012512.GJ22188@codeaurora.org> <20160112051058.GJ1084@ubuntu> <20160112194537.GZ22188@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-pa0-f52.google.com ([209.85.220.52]:35213 "EHLO mail-pa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751723AbcAMFew (ORCPT ); Wed, 13 Jan 2016 00:34:52 -0500 Received: by mail-pa0-f52.google.com with SMTP id ho8so91392384pac.2 for ; Tue, 12 Jan 2016 21:34:52 -0800 (PST) Content-Disposition: inline In-Reply-To: <20160112194537.GZ22188@codeaurora.org> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Stephen Boyd Cc: Rafael Wysocki , linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, nm@ti.com On 12-01-16, 11:45, Stephen Boyd wrote: > > + * Locking: This function takes rcu_read_lock(). > > False. Yeah, I realized it and fixed it right after sending it: + * Locking: This function internally uses mutex locks to keep the integrity of + * the internal data structures. Callers should ensure that this function is + * *NOT* called under RCU protection or in contexts where mutex cannot be + * locked. > > + /* > > + * Hold our list modification lock here as regulator_set_voltage_time() > > + * can possibly take another mutex, which isn't allowed within rcu > > + * locks. > > + */ > > + mutex_lock(&dev_opp_list_lock); > > So now we take the list modification mutex. Why can't we > rcu_read_lock(), find the min and max voltage, and then release > the read lock and ask regulator framework for the voltage time? That was possible before this series came in.. > From what I can tell we're just trying to make sure that the list > is stable when iterating through it to find the min/max voltage. Hmm, we have pointer to regulator within the dev-opp struct. If we drop the lock, the dev-opp struct can get freed and regulator will be put just before that. So, we may be using an already freed resource. How do you suggest we fix it ? -- viresh