From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
Wolfram Sang <wsa@kernel.org>,
Linux PM <linux-pm@vger.kernel.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-i2c <linux-i2c@vger.kernel.org>
Subject: Re: RFC: a failing pm_runtime_get increases the refcnt?
Date: Sun, 14 Jun 2020 15:59:15 +0200 [thread overview]
Message-ID: <CAJZ5v0i87NGcy9+kxubScdPDyByr8ypQWcGgBFn+V-wDd69BHQ@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdUadYRNYdJ9JUX90Z1jvtHZmSS4gM+JKft4x-BK2Ry4zQ@mail.gmail.com>
On Sun, Jun 14, 2020 at 12:00 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Andy,
>
> On Sun, Jun 14, 2020 at 11:43 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sun, Jun 14, 2020 at 12:34 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang <wsa@kernel.org> wrote:
> > > > both in the I2C subsystem and also for Renesas drivers I maintain, I am
> > > > starting to get boilerplate patches doing some pm_runtime_put_* variant
> > > > because a failing pm_runtime_get is supposed to increase the ref
> > > > counters? Really? This feels wrong and unintuitive to me.
> > >
> > > Yeah, that is a well known issue with PM (I even have for a long time
> > > a coccinelle script, when I realized myself that there are a lot of
> > > cases like this, but someone else discovered this recently, like
> > > opening a can of worms).
> > >
> > > > I expect there
> > > > has been a discussion around it but I couldn't find it.
> > >
> > > Rafael explained (again) recently this. I can't find it quickly, unfortunately.
> >
> > I _think_ this discussion, but may be it's simple another tentacle of
> > the same octopus.
> > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao.liu@zju.edu.cn/
>
> Thanks, hadn't read that one! (so I was still at -1 from
> http://sweng.the-davies.net/Home/rustys-api-design-manifesto ;-)
>
> So "pm_runtime_put_noidle()" is the (definitive?) one to pair with a
> pm_runtime_get_sync() failure?
If you bail out immediately on errors, then yes, it is.
If you'd rather to something like
ret = pm_runtime_get_sync(dev);
if (ret < 0)
goto fail;
... code depending on PM ...
fail:
pm_runtime_put_autosuspend(dev);
then it will still work correctly.
It actually doesn't matter which pm_runtime_put*() variant you call
after a pm_runtime_get_sync() failure, but the _noidle() is the
simplest one and it is sufficient.
> > > > I wonder why we
> > > > don't fix the code where the incremented refcount is expected for some
> > > > reason.
> > >
> > > The main idea behind API that a lot of drivers do *not* check error
> > > codes from runtime PM, so, we need to keep balance in case of
> > >
> > > pm_runtime_get(...);
> > > ...
> > > pm_runtime_put(...);
>
> I've always[*] considered a pm_runtime_get_sync() failure to be fatal
> (or: cannot happen), and that there's nothing that can be done to
> recover. Hence I never checked the function's return value.
> Was that wrong?
No, it wasn't. It is the right thing to do in the majority of cases.
> [*] at least on Renesas SoCs with Clock and/or Power Domains.
Cheers!
next prev parent reply other threads:[~2020-06-14 13:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-14 9:07 RFC: a failing pm_runtime_get increases the refcnt? Wolfram Sang
2020-06-14 9:34 ` Andy Shevchenko
2020-06-14 9:42 ` Andy Shevchenko
2020-06-14 10:00 ` Geert Uytterhoeven
2020-06-14 10:04 ` Geert Uytterhoeven
2020-06-14 10:44 ` Andy Shevchenko
2020-06-14 12:42 ` Andy Shevchenko
2020-06-14 13:59 ` Rafael J. Wysocki [this message]
2020-06-14 14:07 ` Wolfram Sang
2020-06-30 19:48 ` Wolfram Sang
2020-06-14 13:50 ` Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJZ5v0i87NGcy9+kxubScdPDyByr8ypQWcGgBFn+V-wDd69BHQ@mail.gmail.com \
--to=rafael@kernel.org \
--cc=andy.shevchenko@gmail.com \
--cc=geert@linux-m68k.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=wsa@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.