PM: Fix symbol export for _SIMPLE_ variants of _PM_OPS()
Subsystems (3)
PATCH v3 1/1 PM: Fix symbol export for _SIMPLE_ variants of _PM_OPS()
Currently EXPORT_*_SIMPLE_DEV_PM_OPS() use EXPORT_*_DEV_PM_OPS() set of macros to export dev_pm_ops symbol, which export the symbol in case CONFIG_PM=y but don't take CONFIG_PM_SLEEP into consideration.
Since _SIMPLE_ variants of _PM_OPS() do not include runtime PM handles and are only used in case CONFIG_PM_SLEEP=y, we should not be exporting their dev_pm_ops symbol in case CONFIG_PM_SLEEP=n.
This can be fixed by having two distinct set of export macros for both _RUNTIME_ and _SIMPLE_ variants of _PM_OPS(), such that the export of dev_pm_ops symbol used in each variant depends on CONFIG_PM and CONFIG_PM_SLEEP respectively.
Introduce _DEV_SLEEP_PM_OPS() set of export macros for _SIMPLE_ variants of _PM_OPS(), which export dev_pm_ops symbol only in case CONFIG_PM_SLEEP=y and discard it otherwise.
---
PS: This is a standalone fix and works without updating any drivers.
Changes since v2:
- Drop redundant patches
Changes since v1:
- Update drivers to new set of macros
include/linux/pm.h | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-)
include/linux/pm.h +29 -14
@@ -374,24 +374,39 @@ const struct dev_pm_ops name = { \ RUNTIME_PM_OPS(runtime_suspend_fn, runtime_resume_fn, idle_fn) \ } -#ifdef CONFIG_PM -#define _EXPORT_DEV_PM_OPS (name, license, ns) \ +#define _EXPORT_PM_OPS (name, license, ns) \ const struct dev_pm_ops name; \ __EXPORT_SYMBOL(name, license, ns); \ const struct dev_pm_ops name -#define EXPORT_PM_FN_GPL (name) EXPORT_SYMBOL_GPL(name) -#define EXPORT_PM_FN_NS_GPL (name, ns) EXPORT_SYMBOL_NS_GPL(name, ns) -#else -#define _EXPORT_DEV_PM_OPS (name, license, ns) \ + +#define _PM_OPS (name, license, ns) \ static __maybe_unused const struct dev_pm_ops __static_##name + +#ifdef CONFIG_PM +#define _EXPORT_DEV_PM_OPS (name, license, ns) _EXPORT_PM_OPS(name, license, ns) +#define EXPORT_PM_FN_GPL (name) EXPORT_SYMBOL_GPL(name) +#define EXPORT_PM_FN_NS_GPL (name, ns) EXPORT_SYMBOL_NS_GPL(name, ns) +#else +#define _EXPORT_DEV_PM_OPS (name, license, ns) _PM_OPS(name, license, ns)#define EXPORT_PM_FN_GPL (name)#define EXPORT_PM_FN_NS_GPL (name, ns)#endif -#define EXPORT_DEV_PM_OPS (name) _EXPORT_DEV_PM_OPS(name, "", "") -#define EXPORT_GPL_DEV_PM_OPS (name) _EXPORT_DEV_PM_OPS(name, "GPL", "") -#define EXPORT_NS_DEV_PM_OPS (name, ns) _EXPORT_DEV_PM_OPS(name, "", #ns) -#define EXPORT_NS_GPL_DEV_PM_OPS (name, ns) _EXPORT_DEV_PM_OPS(name, "GPL", #ns) +#ifdef CONFIG_PM_SLEEP +#define _EXPORT_DEV_SLEEP_PM_OPS (name, license, ns) _EXPORT_PM_OPS(name, license, ns) +#else +#define _EXPORT_DEV_SLEEP_PM_OPS (name, license, ns) _PM_OPS(name, license, ns) +#endif + +#define EXPORT_DEV_PM_OPS (name) _EXPORT_DEV_PM_OPS(name, "", "") +#define EXPORT_GPL_DEV_PM_OPS (name) _EXPORT_DEV_PM_OPS(name, "GPL", "") +#define EXPORT_NS_DEV_PM_OPS (name, ns) _EXPORT_DEV_PM_OPS(name, "", #ns) +#define EXPORT_NS_GPL_DEV_PM_OPS (name, ns) _EXPORT_DEV_PM_OPS(name, "GPL", #ns) + +#define EXPORT_DEV_SLEEP_PM_OPS (name) _EXPORT_DEV_SLEEP_PM_OPS(name, "", "") +#define EXPORT_GPL_DEV_SLEEP_PM_OPS (name) _EXPORT_DEV_SLEEP_PM_OPS(name, "GPL", "") +#define EXPORT_NS_DEV_SLEEP_PM_OPS (name, ns) _EXPORT_DEV_SLEEP_PM_OPS(name, "", #ns) +#define EXPORT_NS_GPL_DEV_SLEEP_PM_OPS (name, ns) _EXPORT_DEV_SLEEP_PM_OPS(name, "GPL", #ns)/* * Use this if you want to use the same suspend and resume callbacks for suspend
@@ -404,19 +419,19 @@ const struct dev_pm_ops name = { \ _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL)#define EXPORT_SIMPLE_DEV_PM_OPS (name, suspend_fn, resume_fn) \ - EXPORT_DEV_PM_OPS(name) = { \ + EXPORT_DEV_SLEEP_PM_OPS(name) = { \ SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ }#define EXPORT_GPL_SIMPLE_DEV_PM_OPS (name, suspend_fn, resume_fn) \ - EXPORT_GPL_DEV_PM_OPS(name) = { \ + EXPORT_GPL_DEV_SLEEP_PM_OPS(name) = { \ SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ }#define EXPORT_NS_SIMPLE_DEV_PM_OPS (name, suspend_fn, resume_fn, ns) \ - EXPORT_NS_DEV_PM_OPS(name, ns) = { \ + EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns) = { \ SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ }#define EXPORT_NS_GPL_SIMPLE_DEV_PM_OPS (name, suspend_fn, resume_fn, ns) \ - EXPORT_NS_GPL_DEV_PM_OPS(name, ns) = { \ + EXPORT_NS_GPL_DEV_SLEEP_PM_OPS(name, ns) = { \ SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ }
Signature · 1 line
2.17.1
Hi,
Le mercredi 20 septembre 2023 à 14:11 +0530, Raag Jadav a écrit :
Quoted 23 lines
Currently EXPORT_*_SIMPLE_DEV_PM_OPS() use EXPORT_*_DEV_PM_OPS() set of macros to export
dev_pm_opssymbol, which export the symbol in caseCONFIG_PM=ybut don't takeCONFIG_PM_SLEEPinto consideration.Since _SIMPLE_ variants of
_PM_OPS()do not include runtime PM handles
and are only used in caseCONFIG_PM_SLEEP=y, we should not be exporting
theirdev_pm_opssymbol in caseCONFIG_PM_SLEEP=n.This can be fixed by having two distinct set of export macros for both
_RUNTIME_ and _SIMPLE_ variants of_PM_OPS(), such that the export ofdev_pm_opssymbol used in each variant depends onCONFIG_PMandCONFIG_PM_SLEEPrespectively.Introduce
_DEV_SLEEP_PM_OPS()set of export macros for _SIMPLE_ variants
of_PM_OPS(), which exportdev_pm_opssymbol only in caseCONFIG_PM_SLEEP=y
and discard it otherwise.
Quoted 4 lines
Fixes 34e1ed189fab ("PM: Improve EXPORT_*_DEV_PM_OPS macros")Signed-off-by Raag Jadav <[email protected]>---
PS: This is a standalone fix and works without updating any drivers.
I had to double-check that, to make sure that none of the drivers using these macros also use pm_ptr() instead of pm_sleep_ptr() to access the exported dev_pm_ops.
I did not check extensively but everything seems to use pm_sleep_ptr(), so it looks safe enough.
Quoted 35 lines
Changes since v2:
- Drop redundant patchesChanges since v1:
- Update drivers to new set of macrosinclude/linux/pm.h | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)include/linux/pm.h +0
-0@@ -374,24 +374,39 @@ const struct dev_pm_ops name = { \ RUNTIME_PM_OPS(runtime_suspend_fn, runtime_resume_fn, idle_fn) \ }include/linux/pm.h +0
-2-#ifdef CONFIG_PM -#define _EXPORT_DEV_PM_OPS(name, license,ns) \
+#define _EXPORT_PM_OPS(name, license,
ns) \
const structdev_pm_ops
name; \
__EXPORT_SYMBOL(name, license,
ns); \
const structdev_pm_opsnameinclude/linux/pm.h +0
-2-#define EXPORT_PM_FN_GPL (name) EXPORT_SYMBOL_GPL(name) -#define EXPORT_PM_FN_NS_GPL (name, ns) EXPORT_SYMBOL_NS_GPL(name,ns)
include/linux/pm.h +0
-2-#else -#define _EXPORT_DEV_PM_OPS(name, license,ns) \
include/linux/pm.h
Quoted 2 lines
include/linux/pm.h +2
-0+ +#define _PM_OPS(name, license,
This macro creates a dev_pm_ops that's meant to be garbage-collected by the compiler; so maybe name it _USELESS_PM_OPS() or something like that.
(I could have said that in your v2 patch, sorry)
Cheers,
-Paul
Quoted 85 lines
ns) \
static__maybe_unusedconst structdev_pm_ops__static_##nameinclude/linux/pm.h +3
-0+ +#ifdef CONFIG_PM +#define _EXPORT_DEV_PM_OPS(name, license,ns) _EXPORT_PM_OPS(name, license, ns)
+#define
EXPORT_PM_FN_GPL(name) EXPORT_SYMBOL_GPL(name )
+#define EXPORT_PM_FN_NS_GPL(name,
ns) EXPORT_SYMBOL_NS_GPL(name, ns)include/linux/pm.h +2
-0+#else +#define _EXPORT_DEV_PM_OPS (name, license, ns) _PM_OPS(name,license, ns)
#defineEXPORT_PM_FN_GPL(name)
#defineEXPORT_PM_FN_NS_GPL(name, ns)
#endifinclude/linux/pm.h +0
-2-#define EXPORT_DEV_PM_OPS (name) _EXPORT_DEV_PM_OPS(name, "", "") -#define EXPORT_GPL_DEV_PM_OPS (name) _EXPORT_DEV_PM_OPS(name, "GPL","")
-#defineEXPORT_NS_DEV_PM_OPS(name, ns)_EXPORT_DEV_PM_OPS(name, "", #ns)
-#defineEXPORT_NS_GPL_DEV_PM_OPS(name, ns)_EXPORT_DEV_PM_OPS(name, "GPL", #ns)include/linux/pm.h +2
-0+#ifdef CONFIG_PM_SLEEP +#define _EXPORT_DEV_SLEEP_PM_OPS(name, license,ns) _EXPORT_PM_OPS(name, license, ns)
include/linux/pm.h +2
-0+#else +#define _EXPORT_DEV_SLEEP_PM_OPS (name, license, ns) _PM_OPS(name,license, ns)
include/linux/pm.h +3
-0+#endif + +#defineEXPORT_DEV_PM_OPS(name) _EXPORT_DEV_PM
_OPS(name, "", "")
+#define
EXPORT_GPL_DEV_PM_OPS(name) _EXPORT_DEV_PM_OPS(nam e, "GPL", "")
+#define EXPORT_NS_DEV_PM_OPS(name,
ns) _EXPORT_DEV_PM_OPS(name, "", #ns) +#define EXPORT_NS_GPL_DEV_PM_OPS(name,
ns) _EXPORT_DEV_PM_OPS(name, "GPL", #ns)include/linux/pm.h +2
-0+ +#defineEXPORT_DEV_SLEEP_PM_OPS(name) _EXPORT_DEV_SLEEP_PM_O
PS(name, "", "")
+#define
EXPORT_GPL_DEV_SLEEP_PM_OPS(name) _EXPORT_DEV_SLEEP_PM_OPS(name, "GPL", "")
+#define EXPORT_NS_DEV_SLEEP_PM_OPS(name,
ns) _EXPORT_DEV_SLEEP_PM_OPS(name, "", #ns) +#define EXPORT_NS_GPL_DEV_SLEEP_PM_OPS(name,
ns) _EXPORT_DEV_SLEEP_PM_OPS(name, "GPL", #ns)
/*
* Use this if you want to use the same suspend and resume callbacks for suspendinclude/linux/pm.h +0
-0@@ -404,19 +419,19 @@ const struct dev_pm_ops name = { \ _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL,NULL )
#defineEXPORT_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn)\
- EXPORT_DEV_PM_OPS(name) = { \
+ EXPORT_DEV_SLEEP_PM_OPS(name) = { \
SYSTEM_SLEEP_PM_OPS(suspend_fn,resume_fn) \
}
#defineEXPORT_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn)\
- EXPORT_GPL_DEV_PM_OPS(name) = { \
+ EXPORT_GPL_DEV_SLEEP_PM_OPS(name) = { \
SYSTEM_SLEEP_PM_OPS(suspend_fn,resume_fn) \
}
#defineEXPORT_NS_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)\
- EXPORT_NS_DEV_PM_OPS(name, ns) = { \
+ EXPORT_NS_DEV_SLEEP_PM_OPS(name, ns) = { \
SYSTEM_SLEEP_PM_OPS(suspend_fn,resume_fn) \
}
#defineEXPORT_NS_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)\
- EXPORT_NS_GPL_DEV_PM_OPS(name, ns) = { \
+ EXPORT_NS_GPL_DEV_SLEEP_PM_OPS(name, ns) = { \
SYSTEM_SLEEP_PM_OPS(suspend_fn,resume_fn) \
}
Quoted 29 lines · Paul Cercueil
Le mercredi 20 septembre 2023 à 14:11 +0530, Raag Jadav a écrit :
Quoted 23 lines
Currently EXPORT_*_SIMPLE_DEV_PM_OPS() use EXPORT_*_DEV_PM_OPS() set of macros to export
dev_pm_opssymbol, which export the symbol in caseCONFIG_PM=ybut don't takeCONFIG_PM_SLEEPinto consideration.Since _SIMPLE_ variants of
_PM_OPS()do not include runtime PM handles
and are only used in caseCONFIG_PM_SLEEP=y, we should not be exporting
theirdev_pm_opssymbol in caseCONFIG_PM_SLEEP=n.This can be fixed by having two distinct set of export macros for both
_RUNTIME_ and _SIMPLE_ variants of_PM_OPS(), such that the export ofdev_pm_opssymbol used in each variant depends onCONFIG_PMandCONFIG_PM_SLEEPrespectively.Introduce
_DEV_SLEEP_PM_OPS()set of export macros for _SIMPLE_ variants
of_PM_OPS(), which exportdev_pm_opssymbol only in caseCONFIG_PM_SLEEP=y
and discard it otherwise.Quoted 4 lines
Fixes 34e1ed189fab ("PM: Improve EXPORT_*_DEV_PM_OPS macros")Signed-off-by Raag Jadav <[email protected]>---
PS: This is a standalone fix and works without updating any drivers.
Quoted 6 lines
I had to double-check that, to make sure that none of the drivers using these macros also use
pm_ptr()instead ofpm_sleep_ptr()to access the exporteddev_pm_ops.I did not check extensively but everything seems to use
pm_sleep_ptr(), so it looks safe enough.
I have tested it against -rc2 without any problems.
Quoted 32 lines
Quoted 32 lines
Changes since v2:
- Drop redundant patchesChanges since v1:
- Update drivers to new set of macrosinclude/linux/pm.h | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)include/linux/pm.h +0
-0@@ -374,24 +374,39 @@ const struct dev_pm_ops name = { \ RUNTIME_PM_OPS(runtime_suspend_fn, runtime_resume_fn, idle_fn) \ }include/linux/pm.h +0
-2-#ifdef CONFIG_PM -#define _EXPORT_DEV_PM_OPS(name, license,ns) \
+#define _EXPORT_PM_OPS(name, license,
ns) \
const structdev_pm_ops
name; \
__EXPORT_SYMBOL(name, license,
ns); \
const structdev_pm_opsnameinclude/linux/pm.h +0
-2-#define EXPORT_PM_FN_GPL (name) EXPORT_SYMBOL_GPL(name) -#define EXPORT_PM_FN_NS_GPL (name, ns) EXPORT_SYMBOL_NS_GPL(name,ns)
-#else
Quoted 8 lines
include/linux/pm.h
Quoted 4 lines
-#define _EXPORT_DEV_PM_OPS(name, license,
ns) \include/linux/pm.h +2
-0+ +#define _PM_OPS(name, license,This macro creates a
dev_pm_opsthat's meant to be garbage-collected by the compiler; so maybe name it_USELESS_PM_OPS()or something like that.
_USELESS_PM_OPS() sounds a bit heavy handed ;)
Gives the impression that the macro itelf is not used anywhere in code.
Something like _DISCARD_PM_OPS() makes more sense.
Raag
Le mercredi 20 septembre 2023 à 12:33 +0300, Raag Jadav a écrit :
Quoted 39 lines · Paul Cercueil
Quoted 38 lines
Le mercredi 20 septembre 2023 à 14:11 +0530, Raag Jadav a écrit :
Quoted 25 lines
Currently EXPORT_*_SIMPLE_DEV_PM_OPS() use EXPORT_*_DEV_PM_OPS() set
of macros to exportdev_pm_opssymbol, which export the symbol in caseCONFIG_PM=ybut don't takeCONFIG_PM_SLEEPinto consideration.Since _SIMPLE_ variants of
_PM_OPS()do not include runtime PM handles
and are only used in caseCONFIG_PM_SLEEP=y, we should not be exporting
theirdev_pm_opssymbol in caseCONFIG_PM_SLEEP=n.This can be fixed by having two distinct set of export macros for both
_RUNTIME_ and _SIMPLE_ variants of_PM_OPS(), such that the export ofdev_pm_opssymbol used in each variant depends onCONFIG_PMandCONFIG_PM_SLEEPrespectively.Introduce
_DEV_SLEEP_PM_OPS()set of export macros for _SIMPLE_ variants
of_PM_OPS(), which exportdev_pm_opssymbol only in caseCONFIG_PM_SLEEP=y
and discard it otherwise.Quoted 5 lines
Fixes 34e1ed189fab ("PM: Improve EXPORT_*_DEV_PM_OPS macros")Signed-off-by Raag Jadav <[email protected]>---
PS: This is a standalone fix and works without updating any drivers.I had to double-check that, to make sure that none of the drivers using
these macros also usepm_ptr()instead ofpm_sleep_ptr()to access the
exporteddev_pm_ops.
Quoted 5 lines
Quoted 3 lines
I did not check extensively but everything seems to use
pm_sleep_ptr(),
so it looks safe enough.I have tested it against -rc2 without any problems.
You'd need to test an "allyesconfig" with CONFIG_PM=y, and CONFIG_PM_SLEEP disabled. Is that what you tested?
Quoted 45 lines
Quoted 36 lines
Quoted 36 lines
Changes since v2:
- Drop redundant patchesChanges since v1:
- Update drivers to new set of macrosinclude/linux/pm.h | 43 +++++++++++++++++++++++++++++-----------
---1 file changed, 29 insertions(+), 14 deletions(-)include/linux/pm.h +0
-0@@ -374,24 +374,39 @@ const struct dev_pm_ops name = { \ RUNTIME_PM_OPS(runtime_suspend_fn, runtime_resume_fn, idle_fn) \ }include/linux/pm.h +0
-2-#ifdef CONFIG_PM -#define _EXPORT_DEV_PM_OPS(name, license,ns) \
+#define _EXPORT_PM_OPS(name, license,
ns) \
const structdev_pm_ops
name; \
__EXPORT_SYMBOL(name, license,
ns); \
const structdev_pm_opsnameinclude/linux/pm.h +0
-2-#define EXPORT_PM_FN_GPL (name) EXPORT_SYMBOL_GPL(name) -#define EXPORT_PM_FN_NS_GPL(name,ns) EXPORT_SYMBOL_NS_GPL(name,
ns)include/linux/pm.h +0
-2-#else -#define _EXPORT_DEV_PM_OPS(name, license,Quoted 8 lines
include/linux/pm.h
Quoted 3 lines
ns) \
include/linux/pm.h +2
-0+ +#define _PM_OPS(name, license,This macro creates a
dev_pm_opsthat's meant to be garbage- collected by
the compiler; so maybe name it_USELESS_PM_OPS()or something like that.
Quoted 5 lines
_USELESS_PM_OPS()sounds a bit heavy handed ;)
Gives the impression that the macro itelf is not used anywhere in code.Something like
_DISCARD_PM_OPS()makes more sense.
Works for me.
Quoted 2 lines
Raag
Cheers,
-Paul
Quoted 40 lines · Paul Cercueil
Le mercredi 20 septembre 2023 à 12:33 +0300, Raag Jadav a écrit :
Quoted 39 lines · Paul Cercueil
Quoted 38 lines
Le mercredi 20 septembre 2023 à 14:11 +0530, Raag Jadav a écrit :
Quoted 25 lines
Currently EXPORT_*_SIMPLE_DEV_PM_OPS() use EXPORT_*_DEV_PM_OPS() set
of macros to exportdev_pm_opssymbol, which export the symbol in caseCONFIG_PM=ybut don't takeCONFIG_PM_SLEEPinto consideration.Since _SIMPLE_ variants of
_PM_OPS()do not include runtime PM handles
and are only used in caseCONFIG_PM_SLEEP=y, we should not be exporting
theirdev_pm_opssymbol in caseCONFIG_PM_SLEEP=n.This can be fixed by having two distinct set of export macros for both
_RUNTIME_ and _SIMPLE_ variants of_PM_OPS(), such that the export ofdev_pm_opssymbol used in each variant depends onCONFIG_PMandCONFIG_PM_SLEEPrespectively.Introduce
_DEV_SLEEP_PM_OPS()set of export macros for _SIMPLE_ variants
of_PM_OPS(), which exportdev_pm_opssymbol only in caseCONFIG_PM_SLEEP=y
and discard it otherwise.Quoted 5 lines
Fixes 34e1ed189fab ("PM: Improve EXPORT_*_DEV_PM_OPS macros")Signed-off-by Raag Jadav <[email protected]>---
PS: This is a standalone fix and works without updating any drivers.I had to double-check that, to make sure that none of the drivers using
these macros also usepm_ptr()instead ofpm_sleep_ptr()to access the
exporteddev_pm_ops.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
I did not check extensively but everything seems to use
pm_sleep_ptr(),
so it looks safe enough.I have tested it against -rc2 without any problems.
You'd need to test an "allyesconfig" with
CONFIG_PM=y, andCONFIG_PM_SLEEPdisabled. Is that what you tested?
Yes, and also validated *.symvers if dev_pm_ops symbol is really discarded.
Seems to work as expected.
Raag