wait: don't wake up freezable wait by freezing fake signal
Commit Message
kernel may try to wake up task with fake signal when freezing task,
if the task is waiting event using wait_event_freezable,
it's better to freeze the wait, instead of breaking it.
otherwise the caller may need to retry the wait,
maybe like below code:
if (rc == -ERESTARTSYS && freezing(current)) {
clear_thread_flag(TIF_SIGPENDING);
goto again;
}
Signed-off-by: TaoJiang <tao.jiang_2@nxp.com>
Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
include/linux/wait.h | 9 +++++++--
kernel/sched/wait.c | 14 ++++++++++++++
2 files changed, 21 insertions(+), 2 deletions(-)
Comments
>From: Peter Zijlstra <peterz@infradead.org>
>Sent: 2023年9月25日 18:08
>To: Ming Qian <ming.qian@nxp.com>
>Cc: mingo@redhat.com; juri.lelli@redhat.com; vincent.guittot@linaro.org;
>dietmar.eggemann@arm.com; rostedt@goodmis.org; bsegall@google.com;
>mgorman@suse.de; bristot@redhat.com; vschneid@redhat.com; Eagle Zhou
><eagle.zhou@nxp.com>; Tao Jiang <tao.jiang_2@nxp.com>; dl-linux-imx
><linux-imx@nxp.com>; linux-kernel@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org
>Subject: [EXT] Re: [PATCH] wait: don't wake up freezable wait by freezing fake
>signal
>
>Caution: This is an external email. Please take care when clicking links or
>opening attachments. When in doubt, report the message using the 'Report
>this email' button
>
>
>On Tue, Aug 22, 2023 at 03:45:47PM +0800, Ming Qian wrote:
>> kernel may try to wake up task with fake signal when freezing task, if
>> the task is waiting event using wait_event_freezable, it's better to
>> freeze the wait, instead of breaking it.
>>
>> otherwise the caller may need to retry the wait, maybe like below
>> code:
>>
>> if (rc == -ERESTARTSYS && freezing(current)) {
>> clear_thread_flag(TIF_SIGPENDING);
>> goto again;
>> }
>
>I'm not sure I get it -- is there an actual problem, or are you trying to optimize
>something?
Hi Peter,
We have a driver that needs to interact with the daemon on nxp IMX8MP-EVK, Initially we use wait_event_interruptible to wait for a response from the daemon, but in our suspend test, sometimes we met failure which was caused by the waiting interrupted.
Then we use wait_event_freezable instead, but the problem remains. The above retry code should fix the problem, but we think it may be a common issue.
So we try to improve the wait_event_freezable, then we can pass our suspend test without any rety.
Ming
@@ -287,6 +287,7 @@ static inline void wake_up_pollfree(struct wait_queue_head *wq_head)
(state & (TASK_INTERRUPTIBLE | TASK_WAKEKILL)))
extern void init_wait_entry(struct wait_queue_entry *wq_entry, int flags);
+bool freezing_wait(struct wait_queue_entry *wq_entry, int state);
/*
* The below macro ___wait_event() has an explicit shadow of the __ret
@@ -314,8 +315,12 @@ extern void init_wait_entry(struct wait_queue_entry *wq_entry, int flags);
break; \
\
if (___wait_is_interruptible(state) && __int) { \
- __ret = __int; \
- goto __out; \
+ if (unlikely(freezing_wait(&__wq_entry, state))) { \
+ clear_thread_flag(TIF_SIGPENDING); \
+ } else { \
+ __ret = __int; \
+ goto __out; \
+ } \
} \
\
cmd; \
@@ -5,6 +5,8 @@
* (C) 2004 Nadia Yvette Chambers, Oracle
*/
+#include <linux/freezer.h>
+
void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *name, struct lock_class_key *key)
{
spin_lock_init(&wq_head->lock);
@@ -484,3 +486,15 @@ int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sy
return default_wake_function(wq_entry, mode, sync, key);
}
EXPORT_SYMBOL(woken_wake_function);
+
+bool freezing_wait(struct wait_queue_entry *wq_entry, int state)
+{
+ if (!(state & TASK_FREEZABLE))
+ return false;
+ if (fatal_signal_pending(wq_entry->private))
+ return false;
+ if (unlikely(freezing(wq_entry->private)))
+ return true;
+ return false;
+}
+EXPORT_SYMBOL(freezing_wait);