| Message ID | 20230822074547.8037-1-ming.qian@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | wait: don't wake up freezable wait by freezing fake signal | expand |
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?
>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
diff --git a/include/linux/wait.h b/include/linux/wait.h index 5ec7739400f4..975ee67f259a 100644 --- a/include/linux/wait.h +++ b/include/linux/wait.h @@ -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; \ diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c index 802d98cf2de3..37c3b2bc1fc9 100644 --- a/kernel/sched/wait.c +++ b/kernel/sched/wait.c @@ -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);
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(-)