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.guittot@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; }
Hi Rafael,
Ping.
Regards, Vincent
On 25 September 2012 15:42, Vincent Guittot vincent.guittot@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.guittot@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
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.
Thanks, Rafael
On 25 September 2012 15:42, Vincent Guittot vincent.guittot@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.guittot@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
Thanks, Rafael
On 25 September 2012 15:42, Vincent Guittot vincent.guittot@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.guittot@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 paulmck@linux.vnet.ibm.com
Thanks, Rafael
On 25 September 2012 15:42, Vincent Guittot vincent.guittot@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.guittot@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, 3 October 2012, Paul E. McKenney paulmck@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 paulmck@linux.vnet.ibm.com
Hi Paul,
Thanks for the review.
Regards, Vincent
Thanks, Rafael
On 25 September 2012 15:42, Vincent Guittot vincent.guittot@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.guittot@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, 3 October 2012, Vincent Guittot vincent.guittot@linaro.org wrote:
On Wednesday, 3 October 2012, Paul E. McKenney paulmck@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 paulmck@linux.vnet.ibm.com
Hi Paul,
Thanks for the review.
Regards, Vincent
Thanks, Rafael
On 25 September 2012 15:42, Vincent Guittot <
vincent.guittot@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.guittot@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 03 of October 2012 21:38:09 Vincent Guittot wrote:
On Wednesday, 3 October 2012, Paul E. McKenney paulmck@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 paulmck@linux.vnet.ibm.com
Hi Paul,
Thanks for the review.
Thanks, I will queue it up for v3.8 when I get back from the current trip.
Rafael
-
Paul E. McKenney -
Rafael J. Wysocki -
Vincent Guittot