linux-kernel - Re: [PATCH v1] PM-runtime: Check supplier

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]

Date:   Mon, 27 Jun 2022 16:14:25 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     peter.wang@...iatek.com
Cc:     matthias.bgg@...il.com, len.brown@...el.com, pavel@....cz,
        rafael@...nel.org, stanley.chu@...iatek.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org, martin.petersen@...cle.com,
        avri.altman@....com, alim.akhtar@...sung.com, jejb@...ux.ibm.com,
        wsd_upstream@...iatek.com, linux-mediatek@...ts.infradead.org,
        chun-hung.wu@...iatek.com, alice.chao@...iatek.com,
        cc.chou@...iatek.com, chaotian.jing@...iatek.com,
        jiajie.hao@...iatek.com, powen.kao@...iatek.com,
        qilin.tan@...iatek.com, lin.gui@...iatek.com,
        tun-yu.yu@...iatek.com
Subject: Re: [PATCH v1] PM-runtime: Check supplier_preactivated before
 release supplier

On Mon, Jun 13, 2022 at 08:07:55PM +0800, peter.wang@...iatek.com wrote:
> From: Peter Wang <peter.wang@...iatek.com>
> 
> With divice link of DL_FLAG_PM_RUNTIME, if consumer call pm_runtime_get_suppliers
> to prevent supplier enter suspend, pm_runtime_release_supplier should
> check supplier_preactivated before let supplier enter suspend.
> 
> If the link is drop or release, bypass check supplier_preactivated.
> 
> Signed-off-by: Peter Wang <peter.wang@...iatek.com>
> ---
>  drivers/base/core.c          |  2 +-
>  drivers/base/power/runtime.c | 15 ++++++++++++---
>  include/linux/pm_runtime.h   |  5 +++--
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 7cd789c4985d..3b9cc559928f 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -486,7 +486,7 @@ static void device_link_release_fn(struct work_struct *work)
>  	/* Ensure that all references to the link object have been dropped. */
>  	device_link_synchronize_removal();
>  
> -	pm_runtime_release_supplier(link, true);
> +	pm_runtime_release_supplier(link, true, true);
>  
>  	put_device(link->consumer);
>  	put_device(link->supplier);
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 676dc72d912d..3c4f425937a1 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -314,10 +314,19 @@ static int rpm_get_suppliers(struct device *dev)
>   * and if @check_idle is set, check if that device is idle (and so it can be
>   * suspended).
>   */
> -void pm_runtime_release_supplier(struct device_link *link, bool check_idle)
> +void pm_runtime_release_supplier(struct device_link *link, bool check_idle,
> +	bool drop)

This is just making this horrible api even worse.  Now there are 2
boolean flags required, 2 more than really should even be here at all.
Every time you see this function being used, you will now have to look
up the definition  to see what it really does.

Please make a new function that calls the internal function with the
flag set properly, so that it is obvious what is happening when the call
is made.

and really, the same thing should be done for the check_idle flag,
that's not good either.

thanks,

greg k-h