On Wednesday, 3 October 2012, Vincent Guittot <vincent.guit...@linaro.org>
wrote:
>
>
> On Wednesday, 3 October 2012, Paul E. McKenney <paul...@linux.vnet.ibm.com>
wrote:
>> 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>
>
> Hi Paul,
>
> Thanks for the review.
>
> Regards,
> Vincent
>>
>>> 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
