* Paul Walmsley <paul@xxxxxxxxx> [080507 17:42]:
>
> Convert pwrdm_mutex to pwrdm_rwsem to avoid trying to relock mutex in the
> event that the pwrdm_for_each() callback function calls something that
> triggers a pwrdm_lookup(). Problem found by Jouni Högander
> <jouni.hogander@xxxxxxxxx>
Pushing.
Tony
>
> Signed-off-by: Paul Walmsley <paul@xxxxxxxxx>
>
> ---
>
> arch/arm/mach-omap2/powerdomain.c | 69 ++++++++++++++++++++-----------------
> 1 files changed, 37 insertions(+), 32 deletions(-)
>
>
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index dd9f40e..e1cfbbb 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -18,7 +18,7 @@
> #include <linux/module.h>
> #include <linux/types.h>
> #include <linux/delay.h>
> -#include <linux/mutex.h>
> +#include <linux/rwsem.h>
> #include <linux/list.h>
> #include <linux/errno.h>
> #include <linux/err.h>
> @@ -37,8 +37,12 @@
> /* pwrdm_list contains all registered struct powerdomains */
> static LIST_HEAD(pwrdm_list);
>
> -/* pwrdm_mutex protects pwrdm_list add and del ops */
> -static DEFINE_MUTEX(pwrdm_mutex);
> +/*
> + * pwrdm_rwsem protects pwrdm_list add and del ops - also reused to
> + * protect pwrdm_clkdms[] during clkdm add/del ops
> + */
> +static DECLARE_RWSEM(pwrdm_rwsem);
> +
>
> /* Private functions */
>
> @@ -135,7 +139,7 @@ int pwrdm_register(struct powerdomain *pwrdm)
> if (!omap_chip_is(pwrdm->omap_chip))
> return -EINVAL;
>
> - mutex_lock(&pwrdm_mutex);
> + down_write(&pwrdm_rwsem);
> if (_pwrdm_lookup(pwrdm->name)) {
> ret = -EEXIST;
> goto pr_unlock;
> @@ -147,7 +151,7 @@ int pwrdm_register(struct powerdomain *pwrdm)
> ret = 0;
>
> pr_unlock:
> - mutex_unlock(&pwrdm_mutex);
> + up_write(&pwrdm_rwsem);
>
> return ret;
> }
> @@ -164,9 +168,9 @@ int pwrdm_unregister(struct powerdomain *pwrdm)
> if (!pwrdm)
> return -EINVAL;
>
> - mutex_lock(&pwrdm_mutex);
> + down_write(&pwrdm_rwsem);
> list_del(&pwrdm->node);
> - mutex_unlock(&pwrdm_mutex);
> + up_write(&pwrdm_rwsem);
>
> pr_debug("powerdomain: unregistered %s\n", pwrdm->name);
>
> @@ -187,9 +191,9 @@ struct powerdomain *pwrdm_lookup(const char *name)
> if (!name)
> return NULL;
>
> - mutex_lock(&pwrdm_mutex);
> + down_read(&pwrdm_rwsem);
> pwrdm = _pwrdm_lookup(name);
> - mutex_unlock(&pwrdm_mutex);
> + up_read(&pwrdm_rwsem);
>
> return pwrdm;
> }
> @@ -198,15 +202,15 @@ struct powerdomain *pwrdm_lookup(const char *name)
> * pwrdm_for_each - call function on each registered clockdomain
> * @fn: callback function *
> *
> - * Call the supplied function for each registered powerdomain.
> - * The callback function can return anything but 0 to bail
> - * out early from the iterator. The callback function is called with
> - * the pwrdm_mutex held, so no powerdomain structure manipulation
> + * Call the supplied function for each registered powerdomain. The
> + * callback function can return anything but 0 to bail out early from
> + * the iterator. The callback function is called with the pwrdm_rwsem
> + * held for reading, so no powerdomain structure manipulation
> * functions should be called from the callback, although hardware
> * powerdomain control functions are fine. Returns the last return
> * value of the callback function, which should be 0 for success or
> - * anything else to indicate failure; or -EINVAL if the function pointer
> - * is null.
> + * anything else to indicate failure; or -EINVAL if the function
> + * pointer is null.
> */
> int pwrdm_for_each(int (*fn)(struct powerdomain *pwrdm))
> {
> @@ -216,13 +220,13 @@ int pwrdm_for_each(int (*fn)(struct powerdomain *pwrdm))
> if (!fn)
> return -EINVAL;
>
> - mutex_lock(&pwrdm_mutex);
> + down_read(&pwrdm_rwsem);
> list_for_each_entry(temp_pwrdm, &pwrdm_list, node) {
> ret = (*fn)(temp_pwrdm);
> if (ret)
> break;
> }
> - mutex_unlock(&pwrdm_mutex);
> + up_read(&pwrdm_rwsem);
>
> return ret;
> }
> @@ -247,7 +251,8 @@ int pwrdm_add_clkdm(struct powerdomain *pwrdm, struct clockdomain *clkdm)
>
> pr_debug("powerdomain: associating clockdomain %s with powerdomain "
> "%s\n", clkdm->name, pwrdm->name);
> - mutex_lock(&pwrdm_mutex);
> +
> + down_write(&pwrdm_rwsem);
>
> for (i = 0; i < PWRDM_MAX_CLKDMS; i++) {
> if (!pwrdm->pwrdm_clkdms[i])
> @@ -273,13 +278,13 @@ int pwrdm_add_clkdm(struct powerdomain *pwrdm, struct clockdomain *clkdm)
> ret = 0;
>
> pac_exit:
> - mutex_unlock(&pwrdm_mutex);
> + up_write(&pwrdm_rwsem);
>
> return ret;
> }
>
> /**
> - * pwrdm_del_clkdm - remove a clockdomain to a powerdomain
> + * pwrdm_del_clkdm - remove a clockdomain from a powerdomain
> * @pwrdm: struct powerdomain * to add the clockdomain to
> * @clkdm: struct clockdomain * to associate with a powerdomain
> *
> @@ -299,25 +304,25 @@ int pwrdm_del_clkdm(struct powerdomain *pwrdm, struct clockdomain *clkdm)
> pr_debug("powerdomain: dissociating clockdomain %s from powerdomain "
> "%s\n", clkdm->name, pwrdm->name);
>
> - mutex_lock(&pwrdm_mutex);
> -
> + down_write(&pwrdm_rwsem);
> +
> for (i = 0; i < PWRDM_MAX_CLKDMS; i++)
> if (pwrdm->pwrdm_clkdms[i] == clkdm)
> break;
> -
> +
> if (i == PWRDM_MAX_CLKDMS) {
> pr_debug("powerdomain: clkdm %s not associated with pwrdm "
> "%s ?!\n", clkdm->name, pwrdm->name);
> ret = -ENOENT;
> goto pdc_exit;
> }
> -
> +
> pwrdm->pwrdm_clkdms[i] = NULL;
> -
> +
> ret = 0;
>
> pdc_exit:
> - mutex_unlock(&pwrdm_mutex);
> + up_write(&pwrdm_rwsem);
>
> return ret;
> }
> @@ -330,10 +335,10 @@ pdc_exit:
> * Call the supplied function for each clockdomain in the powerdomain
> * 'pwrdm'. The callback function can return anything but 0 to bail
> * out early from the iterator. The callback function is called with
> - * the pwrdm_mutex held, so no powerdomain structure manipulation
> - * functions should be called from the callback, although hardware
> - * powerdomain control functions are fine. Returns -EINVAL if
> - * presented with invalid pointers; or passes along the last return
> + * the pwrdm_rwsem held for reading, so no powerdomain structure
> + * manipulation functions should be called from the callback, although
> + * hardware powerdomain control functions are fine. Returns -EINVAL
> + * if presented with invalid pointers; or passes along the last return
> * value of the callback function, which should be 0 for success or
> * anything else to indicate failure.
> */
> @@ -347,12 +352,12 @@ int pwrdm_for_each_clkdm(struct powerdomain *pwrdm,
> if (!fn)
> return -EINVAL;
>
> - mutex_lock(&pwrdm_mutex);
> + down_read(&pwrdm_rwsem);
>
> for (i = 0; i < PWRDM_MAX_CLKDMS && !ret; i++)
> ret = (*fn)(pwrdm, pwrdm->pwrdm_clkdms[i]);
>
> - mutex_unlock(&pwrdm_mutex);
> + up_read(&pwrdm_rwsem);
>
> return ret;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
