linux-kernel - Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error

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: <CACRpkdYrRi3pa6Gw4_Q+P=WYbv-a27FHmOupKVv5s=yU53RFWA@mail.gmail.com>
Date:   Mon, 12 Apr 2021 00:38:41 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Dinghao Liu <dinghao.liu@....edu.cn>, Kangjie Lu <kjlu@....edu>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        linux-iio <linux-iio@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error

On Sun, Apr 11, 2021 at 5:07 PM Jonathan Cameron <jic23@...nel.org> wrote:
> On Wed,  7 Apr 2021 11:49:27 +0800
> Dinghao Liu <dinghao.liu@....edu.cn> wrote:
>
> > When devm_request_threaded_irq() fails, we should decrease the
> > runtime PM counter to keep the counter balanced. But when
> > iio_device_register() fails, we need not to decrease it because
> > we have already decreased it before.
>
> Whilst agree with your assessment that the code is wrong, I'm not
> totally sure why we need to do the pm_runtime_get_noresume() in
> the first place.   Why do we need to hold the reference for
> the operations going on here?  What can race against this that
> might care about that reference count?

pm_runtime_get_noresume() is increasing the runtime PM
reference without calling the pm_runtime_resume() callback.

It is often called in sequence like this:

    pm_runtime_get_noresume(dev);
    pm_runtime_set_active(dev);
    pm_runtime_enable(dev);

This increases the reference, sets the device as active
and enables runtime PM.

The reason that probe() has activated resources such as
enabling two regulators, and want to leave them on so that
later on pm_runtime_suspend() will disable them, i.e.
handover to runtime PM with the device in resumed state.

I hope this is answering the question, not sure.

Yours,
Linus Walleij