| Message ID | 1441373702-31796-3-git-send-email-daniel.wagner@bmw-carit.de (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote: > Instead encode the FREEZE state via the CPU state we allow the > interesting subsystems (MCE, microcode) to query the power > subsystem directly. A use case, please. > Most notifiers are not interested at all > in this information so rather have explicit calls to freeze_active() > instead adding complexity to the rest of the users of the CPU > notifiers. Why does it has anything to do with CPU notifiers? We don't offline CPUs for suspend-to-idle. > Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Cc: Len Brown <len.brown@intel.com> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: linux-pm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > include/linux/suspend.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/linux/suspend.h b/include/linux/suspend.h > index 5efe743..5e15ade 100644 > --- a/include/linux/suspend.h > +++ b/include/linux/suspend.h > @@ -216,6 +216,11 @@ static inline bool idle_should_freeze(void) > return unlikely(suspend_freeze_state == FREEZE_STATE_ENTER); > } > > +static inline bool freeze_active(void) > +{ > + return unlikely(suspend_freeze_state != FREEZE_STATE_NONE); > +} > + > extern void freeze_set_ops(const struct platform_freeze_ops *ops); > extern void freeze_wake(void); > > @@ -244,6 +249,7 @@ extern int pm_suspend(suspend_state_t state); > static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {} > static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; } > static inline bool idle_should_freeze(void) { return false; } > +static inline bool freeze_active(void) { return false; } > static inline void freeze_set_ops(const struct platform_freeze_ops *ops) {} > static inline void freeze_wake(void) {} > #endif /* !CONFIG_SUSPEND */ >
On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote: > On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote: >> Instead encode the FREEZE state via the CPU state we allow the >> interesting subsystems (MCE, microcode) to query the power >> subsystem directly. > > A use case, please. The motivation for this change is to reduce the complexity in the hotplug code. As tried to point out in the cover letter, the FROZEN bits have only a bunch of users after all those years (2007). So it is worth to have all the notifier users to handle the FROZEN state? Don't know if that counts as use case. >> Most notifiers are not interested at all >> in this information so rather have explicit calls to freeze_active() >> instead adding complexity to the rest of the users of the CPU >> notifiers. > > Why does it has anything to do with CPU notifiers? cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set and so most notifiers are doing switch (actcion ~CPU_TASK_FROZEN) to filter it out because they don't need to handle the system wide ongoing freeze operations. > We don't offline CPUs for suspend-to-idle. Sure. As I said the motivation is to reduce the complexity in the hotplug code. Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, September 07, 2015 10:55:43 AM Daniel Wagner wrote: > On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote: > > On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote: > >> Instead encode the FREEZE state via the CPU state we allow the > >> interesting subsystems (MCE, microcode) to query the power > >> subsystem directly. > > > > A use case, please. > > The motivation for this change is to reduce the complexity in the > hotplug code. As tried to point out in the cover letter, the FROZEN > bits have only a bunch of users after all those years (2007). So it is > worth to have all the notifier users to handle the FROZEN state? > > Don't know if that counts as use case. > > >> Most notifiers are not interested at all > >> in this information so rather have explicit calls to freeze_active() > >> instead adding complexity to the rest of the users of the CPU > >> notifiers. > > > > Why does it has anything to do with CPU notifiers? > > cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set > and so most notifiers are doing > > switch (actcion ~CPU_TASK_FROZEN) > > to filter it out because they don't need to handle the system wide > ongoing freeze operations. > > > We don't offline CPUs for suspend-to-idle. > > Sure. As I said the motivation is to reduce the complexity in the > hotplug code. Well, it looks like I confused two things. Let me look at this again. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, September 07, 2015 10:55:43 AM Daniel Wagner wrote: > On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote: > > On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote: > >> Instead encode the FREEZE state via the CPU state we allow the > >> interesting subsystems (MCE, microcode) to query the power > >> subsystem directly. > > > > A use case, please. > > The motivation for this change is to reduce the complexity in the > hotplug code. As tried to point out in the cover letter, the FROZEN > bits have only a bunch of users after all those years (2007). So it is > worth to have all the notifier users to handle the FROZEN state? > > Don't know if that counts as use case. Well, the code you're changing has nothing to do with CPU hotplug and CPU_TASKS_FROZEN. It is about suspend-to-idle. Please grep for suspend_freeze_state and see what it is used for. There is some confusion in the naming, but that is about the freezing of the whole system, while CPU_TASKS_FROZEN is about the freezing of user space. > >> Most notifiers are not interested at all > >> in this information so rather have explicit calls to freeze_active() > >> instead adding complexity to the rest of the users of the CPU > >> notifiers. > > > > Why does it has anything to do with CPU notifiers? > > cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set > and so most notifiers are doing > > switch (actcion ~CPU_TASK_FROZEN) > > to filter it out because they don't need to handle the system wide > ongoing freeze operations. > > > We don't offline CPUs for suspend-to-idle. > > Sure. As I said the motivation is to reduce the complexity in the > hotplug code. You need to fine a different way to do that. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/07/2015 11:44 PM, Rafael J. Wysocki wrote: > On Monday, September 07, 2015 10:55:43 AM Daniel Wagner wrote: >> On 09/05/2015 04:11 AM, Rafael J. Wysocki wrote: >>> On Friday, September 04, 2015 03:34:55 PM Daniel Wagner wrote: >>>> Instead encode the FREEZE state via the CPU state we allow the >>>> interesting subsystems (MCE, microcode) to query the power >>>> subsystem directly. >>> >>> A use case, please. >> >> The motivation for this change is to reduce the complexity in the >> hotplug code. As tried to point out in the cover letter, the FROZEN >> bits have only a bunch of users after all those years (2007). So it is >> worth to have all the notifier users to handle the FROZEN state? >> >> Don't know if that counts as use case. > > Well, the code you're changing has nothing to do with CPU hotplug and > CPU_TASKS_FROZEN. It is about suspend-to-idle. > > Please grep for suspend_freeze_state and see what it is used for. > > There is some confusion in the naming, but that is about the freezing of > the whole system, while CPU_TASKS_FROZEN is about the freezing of user space. You are right. I got confused by all those frozen/freezing naming scheme. >>>> Most notifiers are not interested at all >>>> in this information so rather have explicit calls to freeze_active() >>>> instead adding complexity to the rest of the users of the CPU >>>> notifiers. >>> >>> Why does it has anything to do with CPU notifiers? >> >> cpu_{down|up} will call the notifiers with the CPU_TASK_FROZEN bit set >> and so most notifiers are doing >> >> switch (actcion ~CPU_TASK_FROZEN) >> >> to filter it out because they don't need to handle the system wide >> ongoing freeze operations. >> >>> We don't offline CPUs for suspend-to-idle. >> >> Sure. As I said the motivation is to reduce the complexity in the >> hotplug code. > > You need to fine a different way to do that. I'll try something else. Thanks for taking the time explaining! cheers, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 5efe743..5e15ade 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -216,6 +216,11 @@ static inline bool idle_should_freeze(void) return unlikely(suspend_freeze_state == FREEZE_STATE_ENTER); } +static inline bool freeze_active(void) +{ + return unlikely(suspend_freeze_state != FREEZE_STATE_NONE); +} + extern void freeze_set_ops(const struct platform_freeze_ops *ops); extern void freeze_wake(void); @@ -244,6 +249,7 @@ extern int pm_suspend(suspend_state_t state); static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {} static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; } static inline bool idle_should_freeze(void) { return false; } +static inline bool freeze_active(void) { return false; } static inline void freeze_set_ops(const struct platform_freeze_ops *ops) {} static inline void freeze_wake(void) {} #endif /* !CONFIG_SUSPEND */
Instead encode the FREEZE state via the CPU state we allow the interesting subsystems (MCE, microcode) to query the power subsystem directly. Most notifiers are not interested at all in this information so rather have explicit calls to freeze_active() instead adding complexity to the rest of the users of the CPU notifiers. Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Len Brown <len.brown@intel.com> Cc: Pavel Machek <pavel@ucw.cz> Cc: linux-pm@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- include/linux/suspend.h | 6 ++++++ 1 file changed, 6 insertions(+)