linux-kernel - Re: [PATCH v1] PM: runtime: Unify error handling during suspend and resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0jmp4PFb6z+K9cGS83CmX=5Ms0F9HSgcpq-VXn=rTdXgg@mail.gmail.com>
Date: Mon, 24 Feb 2025 13:39:14 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Raag Jadav <raag.jadav@...el.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, "Rafael J. Wysocki" <rjw@...ysocki.net>, 
	Linux PM <linux-pm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Alan Stern <stern@...land.harvard.edu>, Ulf Hansson <ulf.hansson@...aro.org>, 
	Oliver Neukum <oneukum@...e.com>, Ajay Agarwal <ajayagarwal@...gle.com>, 
	Brian Norris <briannorris@...gle.com>
Subject: Re: [PATCH v1] PM: runtime: Unify error handling during suspend and resume

On Sun, Feb 23, 2025 at 4:42 PM Raag Jadav <raag.jadav@...el.com> wrote:
>
> On Sun, Feb 23, 2025 at 01:56:07PM +0100, Rafael J. Wysocki wrote:
> > On Sun, Feb 23, 2025 at 8:33 AM Raag Jadav <raag.jadav@...el.com> wrote:
> > >
> > > On Thu, Feb 20, 2025 at 09:18:23PM +0100, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > >
> > > > There is a confusing difference in error handling between rpm_suspend()
> > > > and rpm_resume() related to the special way in which the -EAGAIN and
> > > > -EBUSY error values are treated by the former.  Also, converting
> > > > -EACCES coming from the callback to an I/O error, which it quite likely
> > > > is not, may confuse runtime PM users a bit.
> > > >
> > > > To address the above, modify rpm_callback() to convert -EACCES coming
> > > > from the driver to -EAGAIN and to set power.runtime_error only if the
> > > > return value is not -EAGAIN or -EBUSY.
> > > >
> > > > This will cause the error handling in rpm_resume() and rpm_suspend() to
> > > > work consistently, so drop the no longer needed -EAGAIN or -EBUSY
> > > > special case from the latter and make it retry autosuspend if
> > > > power.runtime_error is unset.
> > > >
> > > > Link: https://lore.kernel.org/linux-pm/20220620144231.GA23345@axis.com/
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > > ---
> > > >  drivers/base/power/runtime.c |   34 ++++++++++++++++++----------------
> > > >  1 file changed, 18 insertions(+), 16 deletions(-)
> > > >
> > > > --- a/drivers/base/power/runtime.c
> > > > +++ b/drivers/base/power/runtime.c
> > > > @@ -448,8 +448,13 @@
> > > >               retval = __rpm_callback(cb, dev);
> > > >       }
> > > >
> > > > -     dev->power.runtime_error = retval;
> > > > -     return retval != -EACCES ? retval : -EIO;
> > > > +     if (retval == -EACCES)
> > > > +             retval = -EAGAIN;
> > >
> > > While this is one way to address the problem, are we opening the door
> > > to changing error codes when convenient? This might lead to different
> > > kind of confusion from user standpoint.
> >
> > Are you saying that if a mistake was made sufficiently long ago, it
> > can't be fixed any more because someone may be confused?
>
> Nothing against the fix but "sufficiently long ago" is why we might
> have users that rely on it. As long as we don't break anything I don't
> see a problem.
>
> Messing with error codes is usually received with mixed feelings and
> coming across such a code raises more questions than answers. Perhaps a
> small explanation might do the trick?

Do you mean an explanation why -EACCES needs to be converted to something else?

That's because -EACCES has a special meaning in runtime PM: it means
that runtime PM is disabled for the given device.

Thanks!