lkml.org 
[lkml]   [2011]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 2/3] PM / Runtime: Don't run callbacks under lock for power.irq_safe set
    From
    On Sun, Sep 25, 2011 at 5:25 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
    > From: Rafael J. Wysocki <rjw@sisk.pl>
    >
    > The rpm_suspend() and rpm_resume() routines execute subsystem or PM
    > domain callbacks under power.lock if power.irq_safe is set for the
    > given device.  This is inconsistent with that rpm_idle() does after
    > commit 02b2677 (PM / Runtime: Allow _put_sync() from
    > interrupts-disabled context) and is problematic for subsystems and PM
    > domains wanting to use power.lock for synchronization in their
    > runtime PM callbacks.
    >
    > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

    Reviewed-by: Ming Lei <tom.leiming@gmail.com>

    > ---
    >  drivers/base/power/runtime.c |   68 +++++++++++++++++++++++++++++--------------
    >  1 file changed, 46 insertions(+), 22 deletions(-)
    >
    > Index: linux/drivers/base/power/runtime.c
    > ===================================================================
    > --- linux.orig/drivers/base/power/runtime.c
    > +++ linux/drivers/base/power/runtime.c
    > @@ -155,6 +155,31 @@ static int rpm_check_suspend_allowed(str
    >  }
    >
    >  /**
    > + * __rpm_callback - Run a given runtime PM callback for a given device.
    > + * @cb: Runtime PM callback to run.
    > + * @dev: Device to run the callback for.
    > + */
    > +static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
    > +       __releases(&dev->power.lock) __acquires(&dev->power.lock)
    > +{
    > +       int retval;
    > +
    > +       if (dev->power.irq_safe)
    > +               spin_unlock(&dev->power.lock);
    > +       else
    > +               spin_unlock_irq(&dev->power.lock);
    > +
    > +       retval = cb(dev);
    > +
    > +       if (dev->power.irq_safe)
    > +               spin_lock(&dev->power.lock);
    > +       else
    > +               spin_lock_irq(&dev->power.lock);
    > +
    > +       return retval;
    > +}
    > +
    > +/**
    >  * rpm_idle - Notify device bus type if the device can be suspended.
    >  * @dev: Device to notify the bus type about.
    >  * @rpmflags: Flag bits.
    > @@ -225,19 +250,8 @@ static int rpm_idle(struct device *dev,
    >        else
    >                callback = NULL;
    >
    > -       if (callback) {
    > -               if (dev->power.irq_safe)
    > -                       spin_unlock(&dev->power.lock);
    > -               else
    > -                       spin_unlock_irq(&dev->power.lock);
    > -
    > -               callback(dev);
    > -
    > -               if (dev->power.irq_safe)
    > -                       spin_lock(&dev->power.lock);
    > -               else
    > -                       spin_lock_irq(&dev->power.lock);
    > -       }
    > +       if (callback)
    > +               __rpm_callback(callback, dev);
    >
    >        dev->power.idle_notification = false;
    >        wake_up_all(&dev->power.wait_queue);
    > @@ -252,22 +266,14 @@ static int rpm_idle(struct device *dev,
    >  * @dev: Device to run the callback for.
    >  */
    >  static int rpm_callback(int (*cb)(struct device *), struct device *dev)
    > -       __releases(&dev->power.lock) __acquires(&dev->power.lock)
    >  {
    >        int retval;
    >
    >        if (!cb)
    >                return -ENOSYS;
    >
    > -       if (dev->power.irq_safe) {
    > -               retval = cb(dev);
    > -       } else {
    > -               spin_unlock_irq(&dev->power.lock);
    > -
    > -               retval = cb(dev);
    > +       retval = __rpm_callback(cb, dev);
    >
    > -               spin_lock_irq(&dev->power.lock);
    > -       }
    >        dev->power.runtime_error = retval;
    >        return retval != -EACCES ? retval : -EIO;
    >  }
    > @@ -347,6 +353,15 @@ static int rpm_suspend(struct device *de
    >                        goto out;
    >                }
    >
    > +               if (dev->power.irq_safe) {
    > +                       spin_unlock(&dev->power.lock);
    > +
    > +                       cpu_relax();
    > +
    > +                       spin_lock(&dev->power.lock);
    > +                       goto repeat;
    > +               }
    > +
    >                /* Wait for the other suspend running in parallel with us. */
    >                for (;;) {
    >                        prepare_to_wait(&dev->power.wait_queue, &wait,
    > @@ -496,6 +511,15 @@ static int rpm_resume(struct device *dev
    >                        goto out;
    >                }
    >
    > +               if (dev->power.irq_safe) {
    > +                       spin_unlock(&dev->power.lock);
    > +
    > +                       cpu_relax();
    > +
    > +                       spin_lock(&dev->power.lock);
    > +                       goto repeat;
    > +               }
    > +
    >                /* Wait for the operation carried out in parallel with us. */
    >                for (;;) {
    >                        prepare_to_wait(&dev->power.wait_queue, &wait,
    >
    >


    thanks,
    --
    Ming Lei
    --
    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/

    \
     
     \ /
      Last update: 2011-09-25 10:27    [W:0.032 / U:59.316 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site