netdev - Re: [PATCH 1/2] PM: runtime: Add a general runtime get sync operation to deal with usage counter

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: <CAJZ5v0hpNNAyRuQyMbOE2Lwer_uJbC0uTpnpCBpPNTv54_fxRg@mail.gmail.com>
Date:   Mon, 9 Nov 2020 14:28:41 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     zhangqilong <zhangqilong3@...wei.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        "fugang.duan@....com" <fugang.duan@....com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH 1/2] PM: runtime: Add a general runtime get sync operation
 to deal with usage counter

On Mon, Nov 9, 2020 at 2:24 PM zhangqilong <zhangqilong3@...wei.com> wrote:
>
> Hi
> >
> > On Mon, Nov 9, 2020 at 9:05 AM Zhang Qilong <zhangqilong3@...wei.com>
> > wrote:
> > >
> > > In many case, we need to check return value of pm_runtime_get_sync,
> > > but it brings a trouble to the usage counter processing. Many callers
> > > forget to decrease the usage counter when it failed. It has been
> > > discussed a lot[0][1]. So we add a function to deal with the usage
> > > counter for better coding.
> > >
> > > [0]https://lkml.org/lkml/2020/6/14/88
> > > [1]https://patchwork.ozlabs.org/project/linux-tegra/patch/202005200951
> > > 48.10995-1-dinghao.liu@....edu.cn/
> > > Signed-off-by: Zhang Qilong <zhangqilong3@...wei.com>
> > > ---
> > >  include/linux/pm_runtime.h | 32 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >
> > > diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> > > index 4b708f4e8eed..2b0af5b1dffd 100644
> > > --- a/include/linux/pm_runtime.h
> > > +++ b/include/linux/pm_runtime.h
> > > @@ -386,6 +386,38 @@ static inline int pm_runtime_get_sync(struct device
> > *dev)
> > >         return __pm_runtime_resume(dev, RPM_GET_PUT);  }
> > >
> > > +/**
> > > + * gene_pm_runtime_get_sync - Bump up usage counter of a device and
> > resume it.
> > > + * @dev: Target device.
> >
> > The force argument is not documented.
>
> (1) Good catch, I will add it in next version.
>
> >
> > > + *
> > > + * Increase runtime PM usage counter of @dev first, and carry out
> > > + runtime-resume
> > > + * of it synchronously. If __pm_runtime_resume return negative
> > > + value(device is in
> > > + * error state) or return positive value(the runtime of device is
> > > + already active)
> > > + * with force is true, it need decrease the usage counter of the
> > > + device when
> > > + * return.
> > > + *
> > > + * The possible return values of this function is zero or negative value.
> > > + * zero:
> > > + *    - it means success and the status will store the resume operation
> > status
> > > + *      if needed, the runtime PM usage counter of @dev remains
> > incremented.
> > > + * negative:
> > > + *    - it means failure and the runtime PM usage counter of @dev has
> > been
> > > + *      decreased.
> > > + * positive:
> > > + *    - it means the runtime of the device is already active before that. If
> > > + *      caller set force to true, we still need to decrease the usage
> > counter.
> >
> > Why is this needed?
>
> (2) If caller set force, it means caller will return even the device has already been active
> (__pm_runtime_resume return positive value) after calling gene_pm_runtime_get_sync,
> we still need to decrease the usage count.

But who needs this?

I don't think that it is a good idea to complicate the API this way.