Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <12079576.O9o76ZdvQC@kreacher> <CAPDyKFpVMnxCd9bMg=Wd7_TpSsNM6699NG0QLz8q=KWXk9G9hw@mail.gmail.com>
In-Reply-To: <CAPDyKFpVMnxCd9bMg=Wd7_TpSsNM6699NG0QLz8q=KWXk9G9hw@mail.gmail.com>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Fri, 23 Sep 2022 17:52:58 +0200
Message-ID: <CAJZ5v0h5atuB5mdedfste5EiZKRFgz+7QGQUgWs+=sme4EKbig@mail.gmail.com>
Subject: Re: [PATCH] PM: runtime: Return -EINPROGRESS from rpm_resume() in the
 RPM_NOWAIT case
To:     Ulf Hansson <ulf.hansson@linaro.org>
Cc:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux PM <linux-pm@vger.kernel.org>,
        Douglas Anderson <dianders@chromium.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Alan Stern <stern@rowland.harvard.edu>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Sep 23, 2022 at 3:26 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 22 Sept 2022 at 20:04, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > The prospective callers of rpm_resume() passing RPM_NOWAIT to it may
> > be confused when it returns 0 without actually resuming the device
> > which may happen if the device is suspending at the given time and it
> > will only resume when the suspend in progress has completed.  To avoid
> > that confusion, return -EINPROGRESS from rpm_resume() in that case.
> >
> > Since none of the current callers passing RPM_NOWAIT to rpm_resume()
> > check its return value, this change has no functional impact.
>
> Sounds like there are additional improvements that can be made around
> this, right?

This allows RPM_NOWAIT to be used in places where the caller doesn't
want to wait, because it might deadlock or similar, but they still
need to know whether or not the device can be accessed safely.

Or do you mean something else?

> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Looks good to me!
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Thanks!

> > ---
> >  drivers/base/power/runtime.c |    7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > Index: linux-pm/drivers/base/power/runtime.c
> > ===================================================================
> > --- linux-pm.orig/drivers/base/power/runtime.c
> > +++ linux-pm/drivers/base/power/runtime.c
> > @@ -792,10 +792,13 @@ static int rpm_resume(struct device *dev
> >                 DEFINE_WAIT(wait);
> >
> >                 if (rpmflags & (RPM_ASYNC | RPM_NOWAIT)) {
> > -                       if (dev->power.runtime_status == RPM_SUSPENDING)
> > +                       if (dev->power.runtime_status == RPM_SUSPENDING) {
> >                                 dev->power.deferred_resume = true;
> > -                       else
> > +                               if (rpmflags & RPM_NOWAIT)
> > +                                       retval = -EINPROGRESS;
> > +                       } else {
> >                                 retval = -EINPROGRESS;
> > +                       }
> >                         goto out;
> >                 }
> >
> >
> >
> >