Messages in this thread | | | Date | Wed, 20 Jan 2021 13:09:12 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH] opp: Prepare for ->set_opp() helper to work without regulators |
| |
On 19-01-21, 20:16, Dmitry Osipenko wrote: > 19.01.2021 09:35, Viresh Kumar пишет: > > + mutex_lock(&opp_table->lock); > > + opp_table->set_opp_data = data; > > + if (opp_table->sod_supplies) { > > + data->old_opp.supplies = opp_table->sod_supplies; > > + data->new_opp.supplies = opp_table->sod_supplies + > > + opp_table->regulator_count; > > + } > > + mutex_unlock(&opp_table->lock); > > Why do we need all these locks in this patch?
In case dev_pm_opp_set_regulators() and dev_pm_opp_register_set_opp_helper() get called at the same time. Which can actually happen, though is a corner case.
> The OPP API isn't thread-safe, these locks won't make the API > thread-safe.
I am not sure what you mean by that, can you please explain ?
> At least both sod_supplies and set_opp() pointers should be > set and unset under the lock.
The ->set_opp pointer isn't getting used for a comparison and so putting that inside a lock won't get us anything. We are only using set_opp_data and sod_supplies for comparison at both the places and so they need to be updated within the lock.
-- viresh
| |