introduce pm_call() macro to get rid of most #ifdef CONFIG_PM

RFC PATCH In review Anton Vorontsov 5 participants 15 messages Mar 3, 2008

Subsystems (3)

Suspend to RAM Hibernation Power management core

lore

Share & tools

lore

Anton Vorontsov

author

2008-03-03 00:05 UTC #0 lore raw

RFC PATCH 1/1 introduce pm_call() macro to get rid of most #ifdef CONFIG_PM

Currently drivers handle CONFIG_PM this way:

#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif

struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};

With this patch, the code above converts into:

drv_suspend() {}
drv_resume() {}

struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};

GCC will optimize away suspend/resume calls if they're really not used.

Signed-off-by Anton Vorontsov <[email protected]>

---
 include/linux/pm.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

include/linux/pm.h +7 -0

index 015b735..6e0b9c2 100644
@@ -114,6 +114,13 @@ typedef struct pm_message {
 	int event;
 } pm_message_t;
 
+#ifdef CONFIG_PM
+#define pm_call(x) (x)
+#else
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+
 /*
  * Several driver power state transitions are externally visible, affecting
  * the state of pending I/O queues and (for drivers that touch hardware)

Signature · 1 line

1.5.4.3

Arjan van de Ven

2008-03-03 00:08 UTC #1 lore raw

Quoted 20 lines · On Mon, 3 Mar 2008 02:43:08 +0300 Anton Vorontsov <[email protected]>…

Currently drivers handle CONFIG_PM this way:
#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif
struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};
With this patch, the code above converts into:
drv_suspend() {}
drv_resume() {}

Quoted 8 lines

struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};
GCC will optimize away suspend/resume calls if they're really not used.

to be honest, at this point I would think it's time to remove CONFIG_PM, or rather, just make it always be there and just get rid of the ifdefs. We're saving 2 words and a bit of code, but only a case that not even the embedded guys use.

Arjan van de Ven

2008-03-03 00:08 UTC #2 lore raw

In reply to Anton Vorontsov

Quoted 20 lines · On Mon, 3 Mar 2008 02:43:08 +0300 Anton Vorontsov <[email protected]>…

Currently drivers handle CONFIG_PM this way:
#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif
struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};
With this patch, the code above converts into:
drv_suspend() {}
drv_resume() {}

Quoted 8 lines

struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};
GCC will optimize away suspend/resume calls if they're really not used.

Rafael J. Wysocki

2008-03-03 00:29 UTC #3 lore raw

Quoted 28 lines · Arjan van de Ven

Quoted 25 lines · On Mon, 3 Mar 2008 02:43:08 +0300 Anton Vorontsov <[email protected]>…
Currently drivers handle CONFIG_PM this way:
#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif
struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};
With this patch, the code above converts into:
drv_suspend() {}
drv_resume() {}
struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};

Quoted 8 lines

Quoted 3 lines
GCC will optimize away suspend/resume calls if they're really not used.
to be honest, at this point I would think it's time to remove CONFIG_PM, or rather, just make it always be there and just get rid of the ifdefs. We're saving 2 words and a bit of code, but only a case that not even the embedded guys use.

All of the drivers' ->suspend() and ->resume() callbacks currently depend on CONFIG_PM, which they shouldn't.

Still, we're going to introduce new callbacks for suspend/hibernation and the $subject change will probably get us in the way. Also, it won't be necessary afterwards.

Thanks,
Rafael

Rafael J. Wysocki

2008-03-03 00:31 UTC #4 lore raw

In reply to Arjan van de Ven

Quoted 28 lines · Arjan van de Ven

Quoted 25 lines · On Mon, 3 Mar 2008 02:43:08 +0300 Anton Vorontsov <[email protected]>…
Currently drivers handle CONFIG_PM this way:
#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif
struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};
With this patch, the code above converts into:
drv_suspend() {}
drv_resume() {}
struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};

Quoted 8 lines

Quoted 3 lines
GCC will optimize away suspend/resume calls if they're really not used.
to be honest, at this point I would think it's time to remove CONFIG_PM, or rather, just make it always be there and just get rid of the ifdefs. We're saving 2 words and a bit of code, but only a case that not even the embedded guys use.

All of the drivers' ->suspend() and ->resume() callbacks currently depend on CONFIG_PM, which they shouldn't.

Still, we're going to introduce new callbacks for suspend/hibernation and the $subject change will probably get us in the way. Also, it won't be necessary afterwards.

Thanks,
Rafael

Anton Vorontsov

author

2008-03-03 23:26 UTC #5 lore raw

In reply to Arjan van de Ven

Quoted 28 lines · Arjan van de Ven

Quoted 25 lines · On Mon, 3 Mar 2008 02:43:08 +0300 Anton Vorontsov <[email protected]>…
Currently drivers handle CONFIG_PM this way:
#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif
struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};
With this patch, the code above converts into:
drv_suspend() {}
drv_resume() {}
struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};

Quoted 8 lines

Quoted 3 lines
GCC will optimize away suspend/resume calls if they're really not used.
to be honest, at this point I would think it's time to remove CONFIG_PM, or rather,
just make it always be there and just get rid of the ifdefs.

Don't be so CONFIG_PM-centric :-)

$ zcat /proc/config.gz | grep CONFIG_PM
# CONFIG_PM is not set

This is machine I use at home.

Quoted 2 lines

We're saving 2 words and a bit of code,
but only a case that not even the embedded guys use.

Well, on my ARM machine, with CONFIG_PM=y vmlinux grows by 30KB, half of that is in drivers/. 30KB is 1% of the vmlinux. Is it enough to worth saving? I don't know.

On a x86 box, CONFIG_PM=y grows the vmlinux by 400KB (yeah, it's including ACPI and stuff), though here I didn't count drivers' suspend/resume hooks contribution share.

Signature · 3 lines

Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

Anton Vorontsov

author

2008-03-03 23:48 UTC #6 lore raw

In reply to Arjan van de Ven

Quoted 28 lines · Arjan van de Ven

Quoted 25 lines · On Mon, 3 Mar 2008 02:43:08 +0300 Anton Vorontsov <[email protected]>…
Currently drivers handle CONFIG_PM this way:
#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif
struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};
With this patch, the code above converts into:
drv_suspend() {}
drv_resume() {}
struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};

Quoted 8 lines

Quoted 3 lines
GCC will optimize away suspend/resume calls if they're really not used.
to be honest, at this point I would think it's time to remove CONFIG_PM, or rather,
just make it always be there and just get rid of the ifdefs.

Don't be so CONFIG_PM-centric :-)

$ zcat /proc/config.gz | grep CONFIG_PM
# CONFIG_PM is not set

This is machine I use at home.

Quoted 2 lines

We're saving 2 words and a bit of code,
but only a case that not even the embedded guys use.

Well, on my ARM machine, with CONFIG_PM=y vmlinux grows by 30KB, half of that is in drivers/. 30KB is 1% of the vmlinux. Is it enough to worth saving? I don't know.

On a x86 box, CONFIG_PM=y grows the vmlinux by 400KB (yeah, it's including ACPI and stuff), though here I didn't count drivers' suspend/resume hooks contribution share.

Signature · 3 lines

Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

Pavel Machek

2008-03-03 14:18 UTC #7 lore raw

In reply to Anton Vorontsov

Hi!

Quoted 5 lines

include/linux/pm.h +0 -0
index 015b735..6e0b9c2 100644

include/linux/pm.h typedef struct pm_message { · line 123

Quoted 10 lines

include/linux/pm.h +7 -0

@@ -114,6 +114,13 @@ typedef struct pm_message {
 	int event;
 } pm_message_t;
 
+#ifdef CONFIG_PM
+#define pm_call(x) (x)
+#else
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+

This is evil. Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2008-03-03 14:45 UTC #8 lore raw

In reply to Anton Vorontsov

Hi!

Quoted 5 lines

include/linux/pm.h +0 -0
index 015b735..6e0b9c2 100644

include/linux/pm.h typedef struct pm_message { · line 123

Quoted 10 lines

include/linux/pm.h +7 -0

@@ -114,6 +114,13 @@ typedef struct pm_message {
 	int event;
 } pm_message_t;
 
+#ifdef CONFIG_PM
+#define pm_call(x) (x)
+#else
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+

This is evil. Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Anton Vorontsov

author

2008-03-03 22:42 UTC #9 lore raw

Quoted 10 lines · Pavel Machek

Hi!

Quoted 8 lines

include/linux/pm.h +0 -0

index 015b735..6e0b9c2 100644
@@ -114,6 +114,13 @@ typedef struct pm_message {
 	int event;
 } pm_message_t;

Quoted 10 lines

include/linux/pm.h typedef struct pm_message { · line 123
Quoted 7 lines
include/linux/pm.h +7 -0
+#ifdef CONFIG_PM
+#define pm_call(x) (x)
+#else
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+
This is evil. Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.

No problem. Let's use __maybe_unsed then, it gives the same effect.

- - - -
Subject: introduce pm_call() macro to get rid of most #ifdef CONFIG_PM

Currently drivers handle CONFIG_PM this way:

#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif

struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};

With this patch, the code above converts into:

__maybe_unused drv_suspend() {}
__maybe_unused drv_resume() {}

struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};

GCC will optimize away suspend/resume calls if they're really not used.

Signed-off-by Anton Vorontsov <[email protected]>

---
 include/linux/pm.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

include/linux/pm.h +6 -0

index 015b735..a3932c7 100644
@@ -114,6 +114,12 @@ typedef struct pm_message {
 	int event;
 } pm_message_t;
 
+#ifdef CONFIG_PM
+#define pm_call(x) (x)
+#else
+#define pm_call(x) NULL
+#endif /* CONFIG_PM */
+
 /*
  * Several driver power state transitions are externally visible, affecting
  * the state of pending I/O queues and (for drivers that touch hardware)

Signature · 1 line

1.5.4.3

David Brownell

2008-03-03 22:56 UTC #10 lore raw

In reply to Pavel Machek

Quoted 6 lines · Pavel Machek

include/linux/pm.h typedef struct pm_message { · line 123
Quoted 4 lines
include/linux/pm.h +4 -0
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+
This is evil.

Ugly, yes. It might merit some casts too.

Quoted 2 lines

Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.

It'll be a constant expression; it certainly ought to be.

David Brownell

2008-03-03 22:56 UTC #11 lore raw

In reply to Pavel Machek

Quoted 6 lines · Pavel Machek

include/linux/pm.h typedef struct pm_message { · line 123
Quoted 4 lines
include/linux/pm.h +4 -0
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+
This is evil.

Ugly, yes. It might merit some casts too.

Quoted 2 lines

Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.

It'll be a constant expression; it certainly ought to be.

Pavel Machek

2008-03-03 23:16 UTC #12 lore raw

Hi!

Quoted 7 lines

Quoted 6 lines
include/linux/pm.h typedef struct pm_message { · line 123
Quoted 4 lines
include/linux/pm.h +4 -0
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+
This is evil.

Quoted 7 lines

Ugly, yes. It might merit some casts too.
Quoted 2 lines
Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.
It'll be a constant expression; it certainly ought to be.

Well, we have just fooler compiler to think that x is used, and now we want it to realize that x is unused and compile it out?

Maybe it is smart enough, but...
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2008-03-03 23:16 UTC #13 lore raw

In reply to David Brownell

Hi!

Quoted 7 lines

Quoted 6 lines
include/linux/pm.h typedef struct pm_message { · line 123
Quoted 4 lines
include/linux/pm.h +4 -0
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+
This is evil.

Quoted 7 lines

Ugly, yes. It might merit some casts too.
Quoted 2 lines
Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.
It'll be a constant expression; it certainly ought to be.

Well, we have just fooler compiler to think that x is used, and now we want it to realize that x is unused and compile it out?

Maybe it is smart enough, but...
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Anton Vorontsov

author

2008-03-03 23:04 UTC #14 lore raw

In reply to Pavel Machek

Quoted 10 lines · Pavel Machek

Hi!

Quoted 8 lines

include/linux/pm.h +0 -0

index 015b735..6e0b9c2 100644
@@ -114,6 +114,13 @@ typedef struct pm_message {
 	int event;
 } pm_message_t;

Quoted 10 lines

include/linux/pm.h typedef struct pm_message { · line 123
Quoted 7 lines
include/linux/pm.h +7 -0
+#ifdef CONFIG_PM
+#define pm_call(x) (x)
+#else
+/* avoid `defined but not used' warning */
+#define pm_call(x) ((x) - 1 ? NULL : NULL)
+#endif /* CONFIG_PM */
+
This is evil. Maybe your gcc is smart enough to optimize this away, but I'm not sure mine is.

No problem. Let's use __maybe_unsed then, it gives the same effect.

- - - -
Subject: introduce pm_call() macro to get rid of most #ifdef CONFIG_PM

Currently drivers handle CONFIG_PM this way:

#ifdef CONFIG_PM
drv_suspend() {}
drv_resume() {}
#else
#define drv_suspend NULL
#define drv_resume NULL
#endif

struct driver drv = {
.suspend = drv_suspend,
.resume = drv_resume,
};

With this patch, the code above converts into:

__maybe_unused drv_suspend() {}
__maybe_unused drv_resume() {}

struct driver drv = {
.suspend = pm_call(drv_suspend),
.resume = pm_call(drv_resume),
};

GCC will optimize away suspend/resume calls if they're really not used.

Signed-off-by Anton Vorontsov <[email protected]>

---
 include/linux/pm.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

include/linux/pm.h +6 -0

index 015b735..a3932c7 100644
@@ -114,6 +114,12 @@ typedef struct pm_message {
 	int event;
 } pm_message_t;
 
+#ifdef CONFIG_PM
+#define pm_call(x) (x)
+#else
+#define pm_call(x) NULL
+#endif /* CONFIG_PM */
+
 /*
  * Several driver power state transitions are externally visible, affecting
  * the state of pending I/O queues and (for drivers that touch hardware)

Signature · 1 line

1.5.4.3