linux-kernel - Re: [PATCH V2 5/8] PM / OPP: Add infrastructure to manage multiple regulators

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 9 Nov 2016 17:37:00 -0800
From:   Stephen Boyd <sboyd@...eaurora.org>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Rafael Wysocki <rjw@...ysocki.net>, nm@...com,
        Viresh Kumar <vireshk@...nel.org>,
        linaro-kernel@...ts.linaro.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Vincent Guittot <vincent.guittot@...aro.org>, robh@...nel.org,
        d-gerlach@...com, broonie@...nel.org
Subject: Re: [PATCH V2 5/8] PM / OPP: Add infrastructure to manage multiple
 regulators

On 10/26, Viresh Kumar wrote:
> On 25-10-16, 09:49, Stephen Boyd wrote:
> > On 10/20, Viresh Kumar wrote:
> > > diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
> > > index 37fad2eb0f47..45c70ce07864 100644
> > > --- a/drivers/base/power/opp/core.c
> > > +++ b/drivers/base/power/opp/core.c
> > > @@ -235,21 +237,44 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
> > >  		return 0;
> > >  	}
> > >  
> > > -	reg = opp_table->regulator;
> > > -	if (IS_ERR(reg)) {
> > > +	count = opp_table->regulator_count;
> > > +
> > > +	if (!count) {
> > >  		/* Regulator may not be required for device */
> > >  		rcu_read_unlock();
> > >  		return 0;
> > >  	}
> > >  
> > > -	list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
> > > -		if (!opp->available)
> > > -			continue;
> > > +	size = count * sizeof(*regulators);
> > > +	regulators = kmemdup(opp_table->regulators, size, GFP_KERNEL);
> > 
> > How do we allocate under RCU? Doesn't that spit out big warnings?
> 
> That doesn't. I even tried enabling several locking debug config options.

Please read RCU documentation. From rcu_read_lock() function
documentation:

 In non-preemptible RCU implementations (TREE_RCU and TINY_RCU),
 it is illegal to block while in an RCU read-side critical section.
 In preemptible RCU implementations (TREE_PREEMPT_RCU) in CONFIG_PREEMPT
 kernel builds, RCU read-side critical sections may be preempted,
 but explicit blocking is illegal.  Finally, in preemptible RCU
 implementations in real-time (with -rt patchset) kernel builds, RCU
 read-side critical sections may be preempted and they may also block, but
 only when acquiring spinlocks that are subject to priority inheritance.

I suppose that in certain configurations it will warn and in
others it won't. I thought lockdep would always complain though,
so that's sad it doesn't. At least in some implementations of RCU
rcu_read_lock() is the same as preempt_disable(), which basically
means no sleeping calls like GFP_KERNEL allocations.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project