| Message ID | 20221127-snd-freeze-v8-2-3bc02d09f2ce@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ASoC: SOF: Fix deadlock when shutdown a frozen userspace | expand |
On Thu, Dec 1, 2022 at 12:08 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > Add a way to let the drivers know if the processes are frozen. > > This is needed by drivers that are waiting for processes to end on their > shutdown path. > > Convert pm_freezing into a function and export it, so it can be used by > drivers that are either built-in or modules. > > Cc: stable@vger.kernel.org > Fixes: 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers in .shutdown") > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> Why can't you export the original pm_freezing variable and why is this fixing anything? > --- > include/linux/freezer.h | 3 ++- > kernel/freezer.c | 3 +-- > kernel/power/process.c | 24 ++++++++++++++++++++---- > 3 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/include/linux/freezer.h b/include/linux/freezer.h > index b303472255be..3413c869d68b 100644 > --- a/include/linux/freezer.h > +++ b/include/linux/freezer.h > @@ -13,7 +13,7 @@ > #ifdef CONFIG_FREEZER > DECLARE_STATIC_KEY_FALSE(freezer_active); > > -extern bool pm_freezing; /* PM freezing in effect */ > +bool pm_freezing(void); > extern bool pm_nosig_freezing; /* PM nosig freezing in effect */ > > /* > @@ -80,6 +80,7 @@ static inline int freeze_processes(void) { return -ENOSYS; } > static inline int freeze_kernel_threads(void) { return -ENOSYS; } > static inline void thaw_processes(void) {} > static inline void thaw_kernel_threads(void) {} > +static inline bool pm_freezing(void) { return false; } > > static inline bool try_to_freeze(void) { return false; } > > diff --git a/kernel/freezer.c b/kernel/freezer.c > index 4fad0e6fca64..2d3530ebdb7e 100644 > --- a/kernel/freezer.c > +++ b/kernel/freezer.c > @@ -20,7 +20,6 @@ EXPORT_SYMBOL(freezer_active); > * indicate whether PM freezing is in effect, protected by > * system_transition_mutex > */ > -bool pm_freezing; > bool pm_nosig_freezing; > > /* protects freezing and frozen transitions */ > @@ -46,7 +45,7 @@ bool freezing_slow_path(struct task_struct *p) > if (pm_nosig_freezing || cgroup_freezing(p)) > return true; > > - if (pm_freezing && !(p->flags & PF_KTHREAD)) > + if (pm_freezing() && !(p->flags & PF_KTHREAD)) > return true; > > return false; > diff --git a/kernel/power/process.c b/kernel/power/process.c > index ddd9988327fe..8a4d0e2c8c20 100644 > --- a/kernel/power/process.c > +++ b/kernel/power/process.c > @@ -108,6 +108,22 @@ static int try_to_freeze_tasks(bool user_only) > return todo ? -EBUSY : 0; > } > > +/* > + * Indicate whether PM freezing is in effect, protected by > + * system_transition_mutex. > + */ > +static bool pm_freezing_internal; > + > +/** > + * pm_freezing - indicate whether PM freezing is in effect. > + * > + */ > +bool pm_freezing(void) > +{ > + return pm_freezing_internal; > +} > +EXPORT_SYMBOL(pm_freezing); Use EXPORT_SYMBOL_GPL() instead, please. > + > /** > * freeze_processes - Signal user space processes to enter the refrigerator. > * The current thread will not be frozen. The same process that calls > @@ -126,12 +142,12 @@ int freeze_processes(void) > /* Make sure this task doesn't get frozen */ > current->flags |= PF_SUSPEND_TASK; > > - if (!pm_freezing) > + if (!pm_freezing()) > static_branch_inc(&freezer_active); > > pm_wakeup_clear(0); > pr_info("Freezing user space processes ... "); > - pm_freezing = true; > + pm_freezing_internal = true; > error = try_to_freeze_tasks(true); > if (!error) { > __usermodehelper_set_disable_depth(UMH_DISABLED); > @@ -187,9 +203,9 @@ void thaw_processes(void) > struct task_struct *curr = current; > > trace_suspend_resume(TPS("thaw_processes"), 0, true); > - if (pm_freezing) > + if (pm_freezing()) > static_branch_dec(&freezer_active); > - pm_freezing = false; > + pm_freezing_internal = false; > pm_nosig_freezing = false; > > oom_killer_enable(); > > --
Hi Rafael On Fri, 2 Dec 2022 at 18:48, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Dec 1, 2022 at 12:08 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > > > Add a way to let the drivers know if the processes are frozen. > > > > This is needed by drivers that are waiting for processes to end on their > > shutdown path. > > > > Convert pm_freezing into a function and export it, so it can be used by > > drivers that are either built-in or modules. > > > > Cc: stable@vger.kernel.org > > Fixes: 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers in .shutdown") > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > Why can't you export the original pm_freezing variable and why is this > fixing anything? Because then any module will be able to modify the content of the variable. The Fixes: is because the last patch on the set is doing a real fix. If you only cherry-pick the last patch on a stable branch, the build will fail. (Also, the zero-day builder complains) Anyway, I think we can hold this patch for a bit. The snd people are discussing if this the way to handle it, or if we should handle .shutdown in a different way. Thanks! > > > --- > > include/linux/freezer.h | 3 ++- > > kernel/freezer.c | 3 +-- > > kernel/power/process.c | 24 ++++++++++++++++++++---- > > 3 files changed, 23 insertions(+), 7 deletions(-) > > > > diff --git a/include/linux/freezer.h b/include/linux/freezer.h > > index b303472255be..3413c869d68b 100644 > > --- a/include/linux/freezer.h > > +++ b/include/linux/freezer.h > > @@ -13,7 +13,7 @@ > > #ifdef CONFIG_FREEZER > > DECLARE_STATIC_KEY_FALSE(freezer_active); > > > > -extern bool pm_freezing; /* PM freezing in effect */ > > +bool pm_freezing(void); > > extern bool pm_nosig_freezing; /* PM nosig freezing in effect */ > > > > /* > > @@ -80,6 +80,7 @@ static inline int freeze_processes(void) { return -ENOSYS; } > > static inline int freeze_kernel_threads(void) { return -ENOSYS; } > > static inline void thaw_processes(void) {} > > static inline void thaw_kernel_threads(void) {} > > +static inline bool pm_freezing(void) { return false; } > > > > static inline bool try_to_freeze(void) { return false; } > > > > diff --git a/kernel/freezer.c b/kernel/freezer.c > > index 4fad0e6fca64..2d3530ebdb7e 100644 > > --- a/kernel/freezer.c > > +++ b/kernel/freezer.c > > @@ -20,7 +20,6 @@ EXPORT_SYMBOL(freezer_active); > > * indicate whether PM freezing is in effect, protected by > > * system_transition_mutex > > */ > > -bool pm_freezing; > > bool pm_nosig_freezing; > > > > /* protects freezing and frozen transitions */ > > @@ -46,7 +45,7 @@ bool freezing_slow_path(struct task_struct *p) > > if (pm_nosig_freezing || cgroup_freezing(p)) > > return true; > > > > - if (pm_freezing && !(p->flags & PF_KTHREAD)) > > + if (pm_freezing() && !(p->flags & PF_KTHREAD)) > > return true; > > > > return false; > > diff --git a/kernel/power/process.c b/kernel/power/process.c > > index ddd9988327fe..8a4d0e2c8c20 100644 > > --- a/kernel/power/process.c > > +++ b/kernel/power/process.c > > @@ -108,6 +108,22 @@ static int try_to_freeze_tasks(bool user_only) > > return todo ? -EBUSY : 0; > > } > > > > +/* > > + * Indicate whether PM freezing is in effect, protected by > > + * system_transition_mutex. > > + */ > > +static bool pm_freezing_internal; > > + > > +/** > > + * pm_freezing - indicate whether PM freezing is in effect. > > + * > > + */ > > +bool pm_freezing(void) > > +{ > > + return pm_freezing_internal; > > +} > > +EXPORT_SYMBOL(pm_freezing); > > Use EXPORT_SYMBOL_GPL() instead, please. > > > + > > /** > > * freeze_processes - Signal user space processes to enter the refrigerator. > > * The current thread will not be frozen. The same process that calls > > @@ -126,12 +142,12 @@ int freeze_processes(void) > > /* Make sure this task doesn't get frozen */ > > current->flags |= PF_SUSPEND_TASK; > > > > - if (!pm_freezing) > > + if (!pm_freezing()) > > static_branch_inc(&freezer_active); > > > > pm_wakeup_clear(0); > > pr_info("Freezing user space processes ... "); > > - pm_freezing = true; > > + pm_freezing_internal = true; > > error = try_to_freeze_tasks(true); > > if (!error) { > > __usermodehelper_set_disable_depth(UMH_DISABLED); > > @@ -187,9 +203,9 @@ void thaw_processes(void) > > struct task_struct *curr = current; > > > > trace_suspend_resume(TPS("thaw_processes"), 0, true); > > - if (pm_freezing) > > + if (pm_freezing()) > > static_branch_dec(&freezer_active); > > - pm_freezing = false; > > + pm_freezing_internal = false; > > pm_nosig_freezing = false; > > > > oom_killer_enable(); > > > > -- > > -- > You received this message because you are subscribed to the Google Groups "Chromeos Kdump" group. > To unsubscribe from this group and stop receiving emails from it, send an email to chromeos-kdump+unsubscribe@google.com. > To view this discussion on the web, visit https://groups.google.com/a/google.com/d/msgid/chromeos-kdump/CAJZ5v0jbKSTQopEoXW9FpqDmAqp6Pn%3D-Om5QP2-7ocuGdq8R9w%40mail.gmail.com. -- Ricardo Ribalda
diff --git a/include/linux/freezer.h b/include/linux/freezer.h index b303472255be..3413c869d68b 100644 --- a/include/linux/freezer.h +++ b/include/linux/freezer.h @@ -13,7 +13,7 @@ #ifdef CONFIG_FREEZER DECLARE_STATIC_KEY_FALSE(freezer_active); -extern bool pm_freezing; /* PM freezing in effect */ +bool pm_freezing(void); extern bool pm_nosig_freezing; /* PM nosig freezing in effect */ /* @@ -80,6 +80,7 @@ static inline int freeze_processes(void) { return -ENOSYS; } static inline int freeze_kernel_threads(void) { return -ENOSYS; } static inline void thaw_processes(void) {} static inline void thaw_kernel_threads(void) {} +static inline bool pm_freezing(void) { return false; } static inline bool try_to_freeze(void) { return false; } diff --git a/kernel/freezer.c b/kernel/freezer.c index 4fad0e6fca64..2d3530ebdb7e 100644 --- a/kernel/freezer.c +++ b/kernel/freezer.c @@ -20,7 +20,6 @@ EXPORT_SYMBOL(freezer_active); * indicate whether PM freezing is in effect, protected by * system_transition_mutex */ -bool pm_freezing; bool pm_nosig_freezing; /* protects freezing and frozen transitions */ @@ -46,7 +45,7 @@ bool freezing_slow_path(struct task_struct *p) if (pm_nosig_freezing || cgroup_freezing(p)) return true; - if (pm_freezing && !(p->flags & PF_KTHREAD)) + if (pm_freezing() && !(p->flags & PF_KTHREAD)) return true; return false; diff --git a/kernel/power/process.c b/kernel/power/process.c index ddd9988327fe..8a4d0e2c8c20 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -108,6 +108,22 @@ static int try_to_freeze_tasks(bool user_only) return todo ? -EBUSY : 0; } +/* + * Indicate whether PM freezing is in effect, protected by + * system_transition_mutex. + */ +static bool pm_freezing_internal; + +/** + * pm_freezing - indicate whether PM freezing is in effect. + * + */ +bool pm_freezing(void) +{ + return pm_freezing_internal; +} +EXPORT_SYMBOL(pm_freezing); + /** * freeze_processes - Signal user space processes to enter the refrigerator. * The current thread will not be frozen. The same process that calls @@ -126,12 +142,12 @@ int freeze_processes(void) /* Make sure this task doesn't get frozen */ current->flags |= PF_SUSPEND_TASK; - if (!pm_freezing) + if (!pm_freezing()) static_branch_inc(&freezer_active); pm_wakeup_clear(0); pr_info("Freezing user space processes ... "); - pm_freezing = true; + pm_freezing_internal = true; error = try_to_freeze_tasks(true); if (!error) { __usermodehelper_set_disable_depth(UMH_DISABLED); @@ -187,9 +203,9 @@ void thaw_processes(void) struct task_struct *curr = current; trace_suspend_resume(TPS("thaw_processes"), 0, true); - if (pm_freezing) + if (pm_freezing()) static_branch_dec(&freezer_active); - pm_freezing = false; + pm_freezing_internal = false; pm_nosig_freezing = false; oom_killer_enable();
Add a way to let the drivers know if the processes are frozen. This is needed by drivers that are waiting for processes to end on their shutdown path. Convert pm_freezing into a function and export it, so it can be used by drivers that are either built-in or modules. Cc: stable@vger.kernel.org Fixes: 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers in .shutdown") Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> sdad --- include/linux/freezer.h | 3 ++- kernel/freezer.c | 3 +-- kernel/power/process.c | 24 ++++++++++++++++++++---- 3 files changed, 23 insertions(+), 7 deletions(-)