On Tue, Oct 02, 2012 at 04:02:05AM +0200, Rafael J. Wysocki wrote: > On Monday 01 of October 2012 15:42:39 Vincent Guittot wrote: > > Hi Rafael, > > > > Ping. > > I'm considering this for v3.8, but I'd like Paul (CCed) to tell me what he > thinks about it.
Looks good to me -- the only thing deferred is the freeing of memory. The only risk is that fast repeated invocation of opp_set_availability() could result in a lot of memory waiting for RCU grace periods, but call_rcu() has code to prevent this. So: Reviewed-by: Paul E. McKenney <paul...@linux.vnet.ibm.com> > Thanks, > Rafael > > > > On 25 September 2012 15:42, Vincent Guittot <vincent.guit...@linaro.org> > > wrote: > > > synchronize_rcu blocks the caller of opp_enable/disbale > > > for a complete grace period. This blocking duration prevents > > > any intensive use of the functions. Replace synchronize_rcu > > > by call_rcu which will call our function for freeing the old > > > opp element. > > > > > > The duration of opp_enable and opp_disable will be no more > > > dependant of the grace period. > > > > > > Signed-off-by: Vincent Guittot <vincent.guit...@linaro.org> > > > --- > > > drivers/base/power/opp.c | 19 ++++++++++++++----- > > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c > > > index ac993ea..49e4626 100644 > > > --- a/drivers/base/power/opp.c > > > +++ b/drivers/base/power/opp.c > > > @@ -64,6 +64,7 @@ struct opp { > > > unsigned long u_volt; > > > > > > struct device_opp *dev_opp; > > > + struct rcu_head head; > > > }; > > > > > > /** > > > @@ -441,6 +442,17 @@ int opp_add(struct device *dev, unsigned long freq, > > > unsigned long u_volt) > > > } > > > > > > /** > > > + * opp_free_rcu() - helper to clear the struct opp when grace period has > > > + * elapsed without blocking the the caller of opp_set_availability > > > + */ > > > +static void opp_free_rcu(struct rcu_head *head) > > > +{ > > > + struct opp *opp = container_of(head, struct opp, head); > > > + > > > + kfree(opp); > > > +} > > > + > > > +/** > > > * opp_set_availability() - helper to set the availability of an opp > > > * @dev: device for which we do this operation > > > * @freq: OPP frequency to modify availability > > > @@ -511,7 +523,7 @@ static int opp_set_availability(struct device *dev, > > > unsigned long freq, > > > > > > list_replace_rcu(&opp->node, &new_opp->node); > > > mutex_unlock(&dev_opp_list_lock); > > > - synchronize_rcu(); > > > + call_rcu(&opp->head, opp_free_rcu); > > > > > > /* Notify the change of the OPP availability */ > > > if (availability_req) > > > @@ -521,13 +533,10 @@ static int opp_set_availability(struct device *dev, > > > unsigned long freq, > > > srcu_notifier_call_chain(&dev_opp->head, > > > OPP_EVENT_DISABLE, > > > new_opp); > > > > > > - /* clean up old opp */ > > > - new_opp = opp; > > > - goto out; > > > + return 0; > > > > > > unlock: > > > mutex_unlock(&dev_opp_list_lock); > > > -out: > > > kfree(new_opp); > > > return r; > > > } > > > -- > > > 1.7.9.5 > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev