* Re: [PATCH v2] PM: hibernate: Explicitly set `PM_SUSPEND_MAX` at hibernate entry
2025-05-08 21:17 [PATCH v2] PM: hibernate: Explicitly set `PM_SUSPEND_MAX` at hibernate entry Mario Limonciello
@ 2025-05-09 8:22 ` kernel test robot
2025-05-09 12:43 ` Rafael J. Wysocki
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2025-05-09 8:22 UTC (permalink / raw)
To: Mario Limonciello, mario.limonciello, rafael, pavel, len.brown
Cc: llvm, oe-kbuild-all, linux-pm
Hi Mario,
kernel test robot noticed the following build errors:
[auto build test ERROR on amd-pstate/linux-next]
[also build test ERROR on amd-pstate/bleeding-edge linus/master v6.15-rc5 next-20250508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-hibernate-Explicitly-set-PM_SUSPEND_MAX-at-hibernate-entry/20250509-051901
base: https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git linux-next
patch link: https://lore.kernel.org/r/20250508211822.2698678-1-superm1%40kernel.org
patch subject: [PATCH v2] PM: hibernate: Explicitly set `PM_SUSPEND_MAX` at hibernate entry
config: i386-buildonly-randconfig-004-20250509 (https://download.01.org/0day-ci/archive/20250509/202505091658.9UZSCXpa-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250509/202505091658.9UZSCXpa-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505091658.9UZSCXpa-lkp@intel.com/
All errors (new ones prefixed by >>):
>> kernel/power/main.c:23:17: error: expected ')'
23 | suspend_state_t pm_suspend_target_state;
| ^
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:51: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
kernel/power/main.c:23:17: note: to match this '('
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:24: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
>> kernel/power/main.c:23:17: error: redefinition of 'suspend_state_t' as different kind of symbol
23 | suspend_state_t pm_suspend_target_state;
| ^
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:34: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
include/linux/suspend.h:34:23: note: previous definition is here
34 | typedef int __bitwise suspend_state_t;
| ^
kernel/power/main.c:24:19: error: expected ')'
24 | EXPORT_SYMBOL_GPL(pm_suspend_target_state);
| ^
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:51: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
kernel/power/main.c:24:19: note: to match this '('
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:24: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
kernel/power/main.c:24:19: error: redefinition of 'suspend_state_t' as different kind of symbol
24 | EXPORT_SYMBOL_GPL(pm_suspend_target_state);
| ^
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:34: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
include/linux/suspend.h:34:23: note: previous definition is here
34 | typedef int __bitwise suspend_state_t;
| ^
>> kernel/power/main.c:24:1: error: pasting formed '__addressable_(', an invalid preprocessing token
24 | EXPORT_SYMBOL_GPL(pm_suspend_target_state);
| ^
include/linux/export.h:84:33: note: expanded from macro 'EXPORT_SYMBOL_GPL'
84 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^
include/linux/export.h:80:38: note: expanded from macro '_EXPORT_SYMBOL'
80 | #define _EXPORT_SYMBOL(sym, license) __EXPORT_SYMBOL(sym, license, "")
| ^
include/linux/export.h:71:2: note: expanded from macro '__EXPORT_SYMBOL'
71 | __ADDRESSABLE(sym) \
| ^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler.h:286:14: note: expanded from macro '___ADDRESSABLE'
286 | __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
| ^
include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^
include/linux/compiler_types.h:83:24: note: expanded from macro '___PASTE'
83 | #define ___PASTE(a,b) a##b
| ^
kernel/power/main.c:24:19: error: expected ')'
24 | EXPORT_SYMBOL_GPL(pm_suspend_target_state);
| ^
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:51: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
kernel/power/main.c:24:19: note: to match this '('
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:24: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
>> kernel/power/main.c:24:1: error: pasting formed ')417', an invalid preprocessing token
24 | EXPORT_SYMBOL_GPL(pm_suspend_target_state);
| ^
include/linux/export.h:84:33: note: expanded from macro 'EXPORT_SYMBOL_GPL'
84 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^
include/linux/export.h:80:38: note: expanded from macro '_EXPORT_SYMBOL'
80 | #define _EXPORT_SYMBOL(sym, license) __EXPORT_SYMBOL(sym, license, "")
| ^
include/linux/export.h:71:2: note: expanded from macro '__EXPORT_SYMBOL'
71 | __ADDRESSABLE(sym) \
| ^
note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^
include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^
include/linux/compiler_types.h:83:24: note: expanded from macro '___PASTE'
83 | #define ___PASTE(a,b) a##b
| ^
>> kernel/power/main.c:24:19: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
24 | EXPORT_SYMBOL_GPL(pm_suspend_target_state);
| ^
include/linux/suspend.h:148:34: note: expanded from macro 'pm_suspend_target_state'
148 | #define pm_suspend_target_state (PM_SUSPEND_ON)
| ^
include/linux/suspend.h:36:34: note: expanded from macro 'PM_SUSPEND_ON'
36 | #define PM_SUSPEND_ON ((__force suspend_state_t) 0)
| ^
>> kernel/power/main.c:24:1: error: expected function body after function declarator
24 | EXPORT_SYMBOL_GPL(pm_suspend_target_state);
| ^
include/linux/export.h:84:33: note: expanded from macro 'EXPORT_SYMBOL_GPL'
84 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^
include/linux/export.h:80:38: note: expanded from macro '_EXPORT_SYMBOL'
80 | #define _EXPORT_SYMBOL(sym, license) __EXPORT_SYMBOL(sym, license, "")
| ^
include/linux/export.h:71:2: note: expanded from macro '__EXPORT_SYMBOL'
71 | __ADDRESSABLE(sym) \
| ^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler.h:286:2: note: expanded from macro '___ADDRESSABLE'
286 | __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
| ^
include/linux/compiler.h:166:68: note: expanded from macro '__UNIQUE_ID'
166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^
<scratch space>:210:1: note: expanded from here
210 | 417
| ^
>> kernel/power/main.c:24:1: error: pasting formed '__export_symbol_(', an invalid preprocessing token
include/linux/export.h:84:33: note: expanded from macro 'EXPORT_SYMBOL_GPL'
84 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^
include/linux/export.h:80:38: note: expanded from macro '_EXPORT_SYMBOL'
80 | #define _EXPORT_SYMBOL(sym, license) __EXPORT_SYMBOL(sym, license, "")
| ^
include/linux/export.h:73:18: note: expanded from macro '__EXPORT_SYMBOL'
73 | asm(__stringify(___EXPORT_SYMBOL(sym, license, ns)))
| ^
include/linux/export.h:29:18: note: expanded from macro '___EXPORT_SYMBOL'
29 | __export_symbol_##sym: ASM_NL \
| ^
10 errors generated.
vim +23 kernel/power/main.c
22
> 23 suspend_state_t pm_suspend_target_state;
> 24 EXPORT_SYMBOL_GPL(pm_suspend_target_state);
25
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] PM: hibernate: Explicitly set `PM_SUSPEND_MAX` at hibernate entry
2025-05-08 21:17 [PATCH v2] PM: hibernate: Explicitly set `PM_SUSPEND_MAX` at hibernate entry Mario Limonciello
2025-05-09 8:22 ` kernel test robot
@ 2025-05-09 12:43 ` Rafael J. Wysocki
1 sibling, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2025-05-09 12:43 UTC (permalink / raw)
To: Mario Limonciello; +Cc: mario.limonciello, rafael, pavel, len.brown, linux-pm
On Thu, May 8, 2025 at 11:18 PM Mario Limonciello <superm1@kernel.org> wrote:
>
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> `pm_debug_messages` only works at suspend time, not hibernate. This is
> because during hibernate the `pm_suspend_target_state` is not set.
>
> Explicitly set pm_suspend_target_state at hibernate time. Since the symbol
> is only enabled under `CONFIG_SUSPEND`, adjust symbol location and allow it
> under `CONFIG_HIBERNATION` too.
No, this is not the right fix.
Apart from the build issue introduced by it as reported by 0-day,
pm_suspend_target_state is suspend-only and setting it during
hibernation is almost guaranteed to confuse things somewhere.
Hibernation needs to be covered in a different way.
I'll post my fix shortly (along with some other changes on top of it).
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v2:
> * Fix for lkp robot failure where no CONFIG_SUSPEND
> ---
> include/linux/suspend.h | 9 ++++++---
> kernel/power/hibernate.c | 2 ++
> kernel/power/main.c | 3 +++
> kernel/power/suspend.c | 2 --
> 4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index da6ebca3ff774..b66be47ef5282 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -142,8 +142,13 @@ struct platform_s2idle_ops {
> void (*end)(void);
> };
>
> -#ifdef CONFIG_SUSPEND
> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_HIBERNATION)
> extern suspend_state_t pm_suspend_target_state;
> +#else
> +#define pm_suspend_target_state (PM_SUSPEND_ON)
> +#endif
> +
> +#ifdef CONFIG_SUSPEND
> extern suspend_state_t mem_sleep_current;
> extern suspend_state_t mem_sleep_default;
>
> @@ -279,8 +284,6 @@ extern bool sync_on_suspend_enabled;
> #else /* !CONFIG_SUSPEND */
> #define suspend_valid_only_mem NULL
>
> -#define pm_suspend_target_state (PM_SUSPEND_ON)
> -
> static inline void pm_suspend_clear_flags(void) {}
> static inline void pm_set_suspend_via_firmware(void) {}
> static inline void pm_set_resume_via_firmware(void) {}
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index f0db9d1896e80..8fb82a62b85f1 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -772,6 +772,7 @@ int hibernate(void)
> }
>
> pr_info("hibernation entry\n");
> + pm_suspend_target_state = PM_SUSPEND_MAX;
> pm_prepare_console();
> error = pm_notifier_call_chain_robust(PM_HIBERNATION_PREPARE, PM_POST_HIBERNATION);
> if (error)
> @@ -852,6 +853,7 @@ int hibernate(void)
> hibernate_release();
> Unlock:
> unlock_system_sleep(sleep_flags);
> + pm_suspend_target_state = PM_SUSPEND_ON;
> pr_info("hibernation exit\n");
>
> return error;
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 97746f08b762b..0440644695c76 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -20,6 +20,9 @@
>
> #include "power.h"
>
> +suspend_state_t pm_suspend_target_state;
> +EXPORT_SYMBOL_GPL(pm_suspend_target_state);
> +
> #ifdef CONFIG_PM_SLEEP
> /*
> * The following functions are used by the suspend/hibernate code to temporarily
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 8eaec4ab121d4..08cb71991795d 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -48,8 +48,6 @@ const char *mem_sleep_states[PM_SUSPEND_MAX];
>
> suspend_state_t mem_sleep_current = PM_SUSPEND_TO_IDLE;
> suspend_state_t mem_sleep_default = PM_SUSPEND_MAX;
> -suspend_state_t pm_suspend_target_state;
> -EXPORT_SYMBOL_GPL(pm_suspend_target_state);
>
> unsigned int pm_suspend_global_flags;
> EXPORT_SYMBOL_GPL(pm_suspend_global_flags);
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread