Messages in this thread | | | Date | Fri, 22 Nov 2013 10:50:29 +0100 | From | Daniel Lezcano <> | Subject | Re: [PATCH V2 Resend] cpuidle: free all state kobjects from cpuidle_free_state_kobj() |
| |
On 11/21/2013 04:48 PM, Viresh Kumar wrote: > On 21 November 2013 18:39, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> On Thursday, November 21, 2013 08:54:12 AM Viresh Kumar wrote: >>> Loop for states is currently present on callers side and so is replicated at >>> several places. It would be better to move that inside cpuidle_free_state_kobj() >>> instead. >>> >>> This patch does it. >>> >>> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> >>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> >>> --- >>> drivers/cpuidle/sysfs.c | 23 ++++++++++++----------- >>> 1 file changed, 12 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c >>> index e918b6d..ade31a9 100644 >>> --- a/drivers/cpuidle/sysfs.c >>> +++ b/drivers/cpuidle/sysfs.c >>> @@ -378,12 +378,17 @@ static struct kobj_type ktype_state_cpuidle = { >>> .release = cpuidle_state_sysfs_release, >>> }; >>> >>> -static inline void cpuidle_free_state_kobj(struct cpuidle_device *device, int i) >>> +static inline void cpuidle_free_state_kobj(struct cpuidle_device *device, >>> + int count) >>> { >>> - kobject_put(&device->kobjs[i]->kobj); >>> - wait_for_completion(&device->kobjs[i]->kobj_unregister); >>> - kfree(device->kobjs[i]); >>> - device->kobjs[i] = NULL; >>> + int i; >>> + >>> + for (i = 0; i < count; i++) { >>> + kobject_put(&device->kobjs[i]->kobj); >>> + wait_for_completion(&device->kobjs[i]->kobj_unregister); >>> + kfree(device->kobjs[i]); >>> + device->kobjs[i] = NULL; >>> + } >>> } >>> >>> /** >>> @@ -419,8 +424,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device) >>> return 0; >>> >>> error_state: >>> - for (i = i - 1; i >= 0; i--) >>> - cpuidle_free_state_kobj(device, i); >>> + cpuidle_free_state_kobj(device, i); >> >> Well, doesn't the ordering actually matter? Your patch changes the ordering >> here. > > I don't think it matters. And it was done in reverse order earlier to > save an extra > variable..
Yes, that's correct. Without the reverse order we must declare a variable for the error case to do 'for (j = 0; j < i; j++)'
Thanks -- Daniel
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |