linux-kernel - Re: [PATCH] PM / core: refactor PM

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] [day] [month] [year] [list]

Message-ID: <9e1f4be2-436e-0d60-eb28-44c229a3178c@samsung.com>
Date:   Mon, 14 May 2018 14:07:54 +0200
From:   Andrzej Hajda <a.hajda@...sung.com>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH] PM / core: refactor PM_OPS initializers

On 14.05.2018 10:21, Andrzej Hajda wrote:
> With current implementation of PM_OPS initializers users should annotate
> all PM callbacks with __maybe_unused attribute to prevent compiler from
> complaining in case respective option is not enabled. Using ternary
> operator with IS_ENABLED(symbol) as a condition allows to avoid marking
> these functions with __maybe_unused.
> Solution was tested successfully with multiple versions of gcc since
> 4.9.4 up to 7.2.1. No functional changes has been observed and callbacks
> were compiled out if not used, as before.

The kernel does not compile with the patch applied if the driver defines
PM callback inside "#ifdef CONFIG_PM(_SLEEP)" - initializers assume that
callbacks are defined, even if they are not used.
So if the idea is OK I should figure it out how to make proper
transition, any ideas welcome :)

Regards
Andrzej

>
> Signed-off-by: Andrzej Hajda <a.hajda@...sung.com>
> ---
>  include/linux/pm.h | 61 ++++++++++++++++++----------------------------
>  1 file changed, 24 insertions(+), 37 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index e723b78d8357..59f333922c15 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -313,50 +313,37 @@ struct dev_pm_ops {
>  	int (*runtime_idle)(struct device *dev);
>  };
>  
> -#ifdef CONFIG_PM_SLEEP
> +#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
> +#define PM_PTR(ptr) (IS_ENABLED(CONFIG_PM) ? (ptr) : NULL)
> +
>  #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend = suspend_fn, \
> -	.resume = resume_fn, \
> -	.freeze = suspend_fn, \
> -	.thaw = resume_fn, \
> -	.poweroff = suspend_fn, \
> -	.restore = resume_fn,
> -#else
> -#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> +	.suspend = PM_SLEEP_PTR(suspend_fn), \
> +	.resume = PM_SLEEP_PTR(resume_fn), \
> +	.freeze = PM_SLEEP_PTR(suspend_fn), \
> +	.thaw = PM_SLEEP_PTR(resume_fn), \
> +	.poweroff = PM_SLEEP_PTR(suspend_fn), \
> +	.restore = PM_SLEEP_PTR(resume_fn),
>  
> -#ifdef CONFIG_PM_SLEEP
>  #define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend_late = suspend_fn, \
> -	.resume_early = resume_fn, \
> -	.freeze_late = suspend_fn, \
> -	.thaw_early = resume_fn, \
> -	.poweroff_late = suspend_fn, \
> -	.restore_early = resume_fn,
> -#else
> -#define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> +	.suspend_late = PM_SLEEP_PTR(suspend_fn), \
> +	.resume_early = PM_SLEEP_PTR(resume_fn), \
> +	.freeze_late = PM_SLEEP_PTR(suspend_fn), \
> +	.thaw_early = PM_SLEEP_PTR(resume_fn), \
> +	.poweroff_late = PM_SLEEP_PTR(suspend_fn), \
> +	.restore_early = PM_SLEEP_PTR(resume_fn),
>  
> -#ifdef CONFIG_PM_SLEEP
>  #define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend_noirq = suspend_fn, \
> -	.resume_noirq = resume_fn, \
> -	.freeze_noirq = suspend_fn, \
> -	.thaw_noirq = resume_fn, \
> -	.poweroff_noirq = suspend_fn, \
> -	.restore_noirq = resume_fn,
> -#else
> -#define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> +	.suspend_noirq = PM_SLEEP_PTR(suspend_fn), \
> +	.resume_noirq = PM_SLEEP_PTR(resume_fn), \
> +	.freeze_noirq = PM_SLEEP_PTR(suspend_fn), \
> +	.thaw_noirq = PM_SLEEP_PTR(resume_fn), \
> +	.poweroff_noirq = PM_SLEEP_PTR(suspend_fn), \
> +	.restore_noirq = PM_SLEEP_PTR(resume_fn),
>  
> -#ifdef CONFIG_PM
>  #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> -	.runtime_suspend = suspend_fn, \
> -	.runtime_resume = resume_fn, \
> -	.runtime_idle = idle_fn,
> -#else
> -#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
> -#endif
> +	.runtime_suspend = PM_PTR(suspend_fn), \
> +	.runtime_resume = PM_PTR(resume_fn), \
> +	.runtime_idle = PM_PTR(idle_fn),
>  
>  /*
>   * Use this if you want to use the same suspend and resume callbacks for suspend