I think this patch really needs help with the commit message, something like: wait_queue_func_t accepts 4 arguments (struct wait_queue_entry *wq_entry, unsigned mode, int flags, void *key); In the case of poll and select the wait queue function is pollwake in fs/select.c, this wake function passes the third argument flags as the sync parameter to the default_wake_function defined in kernel/sched/core.c. This argument is passed along to try_to_wake_up which continues to pass down the wake flags to select_task_rq and finally in the case of CFS select_task_rq_fair. In select_task_rq_fair the sync flag is passed down to the wake_affine_* functions in kernel/sched/fair.c which accept and honor the sync flag. Epoll however when reciving the WF_SYNC flag completely drops it on the floor, the wakeup function used by epoll is defined in fs/eventpoll.c, ep_poll_callback. This callback receives a sync flag just like pollwake; however, it never does anything with it. Ultimately it wakes up the waiting task directly using wake_up. This shows that there seems to be a divergence between poll/select and epoll regarding honoring sync wakeups. I have tested this patch through self tests and numerous runs of the perf benchmarks for epoll. All tests past and I did not see any observable performance changes in epoll_wait. Reviewed-by: Brian Geffon <bgeffon@xxxxxxxxxx> Tested-by: Brian Geffon <bgeffon@xxxxxxxxxx> Reported-by: Benoit Lize <lizeb@xxxxxxxxxx> On Thu, Sep 19, 2024 at 8:36 AM Brian Geffon <bgeffon@xxxxxxxxxx> wrote: > > We've also observed this issue on ChromeOS, it seems like it might long-standing epoll bug as it diverges from the behavior of poll. Any chance a maintainer can take a look? > > Thanks > Brian > > On Fri, Apr 26, 2024 at 04:05:48PM +0800, Xuewen Yan wrote: > > Now, the epoll only use wake_up() interface to wake up task. > > However, sometimes, there are epoll users which want to use > > the synchronous wakeup flag to hint the scheduler, such as > > Android binder driver. > > So add a wake_up_sync() define, and use the wake_up_sync() > > when the sync is true in ep_poll_callback(). > > > > Co-developed-by: Jing Xia <jing.xia@xxxxxxxxxx> > > Signed-off-by: Jing Xia <jing.xia@xxxxxxxxxx> > > Signed-off-by: Xuewen Yan <xuewen.yan@xxxxxxxxxx> > > --- > > fs/eventpoll.c | 5 ++++- > > include/linux/wait.h | 1 + > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/fs/eventpoll.c b/fs/eventpoll.c > > index 882b89edc52a..9b815e0a1ac5 100644 > > --- a/fs/eventpoll.c > > +++ b/fs/eventpoll.c > > @@ -1336,7 +1336,10 @@ static int ep_poll_callback(wait_queue_entry_t *wait, unsigned mode, int sync, v > > break; > > } > > } > > - wake_up(&ep->wq); > > + if (sync) > > + wake_up_sync(&ep->wq); > > + else > > + wake_up(&ep->wq); > > } > > if (waitqueue_active(&ep->poll_wait)) > > pwake++; > > diff --git a/include/linux/wait.h b/include/linux/wait.h > > index 8aa3372f21a0..2b322a9b88a2 100644 > > --- a/include/linux/wait.h > > +++ b/include/linux/wait.h > > @@ -221,6 +221,7 @@ void __wake_up_pollfree(struct wait_queue_head *wq_head); > > #define wake_up_all(x) __wake_up(x, TASK_NORMAL, 0, NULL) > > #define wake_up_locked(x) __wake_up_locked((x), TASK_NORMAL, 1) > > #define wake_up_all_locked(x) __wake_up_locked((x), TASK_NORMAL, 0) > > +#define wake_up_sync(x) __wake_up_sync(x, TASK_NORMAL) > > > > #define wake_up_interruptible(x) __wake_up(x, TASK_INTERRUPTIBLE, 1, NULL) > > #define wake_up_interruptible_nr(x, nr) __wake_up(x, TASK_INTERRUPTIBLE, nr, NULL) > > -- > > 2.25.1 > > >
