Re: [RFC PATCH] epoll: Add synchronous wakeup support for ep_poll_callback

[Brian Geffon]

On Tue, Dec 17, 2024 at 10:34 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Dec 17, 2024 at 09:30:51AM -0500, Brian Geffon wrote:
> > On Thu, Dec 12, 2024 at 12:14 PM Brian Geffon <bgeffon@xxxxxxxxxx> wrote:
> > >
> > > On Wed, Oct 16, 2024 at 03:05:38PM -0400, Brian Geffon wrote:
> > > > On Wed, Oct 16, 2024 at 03:10:34PM +0200, Christian Brauner wrote:
> > > > > On Fri, 26 Apr 2024 16:05:48 +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().
> > > > > >
> > > > > > [...]
> > > > >
> > > > > Applied to the vfs.misc branch of the vfs/vfs.git tree.
> > > > > Patches in the vfs.misc branch should appear in linux-next soon.
> > > > >
> > > > > Please report any outstanding bugs that were missed during review in a
> > > > > new review to the original patch series allowing us to drop it.
> > > > >
> > > > > It's encouraged to provide Acked-bys and Reviewed-bys even though the
> > > > > patch has now been applied. If possible patch trailers will be updated.
> > > > >
> > > > > Note that commit hashes shown below are subject to change due to rebase,
> > > > > trailer updates or similar. If in doubt, please check the listed branch.
> > > > >
> > > > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> > > > > branch: vfs.misc
> > > >
> > > > This is a bug that's been present for all of time, so I think we should:
> > > >
> > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > >
> > > This is in as 900bbaae ("epoll: Add synchronous wakeup support for
> > > ep_poll_callback"). How do maintainers feel about:
> > >
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Cc: stable@xxxxxxxxxxxxxxx
> >
> > Dear stable maintainers, this fixes a bug goes all the way back and
> > beyond Linux 2.6.12-rc2. Can you please add this commit to the stable
> > releases?
> >
> > commit 900bbaae67e980945dec74d36f8afe0de7556d5a upstream.
>

Hi Greg,

> How is this a bugfix?  It looks like it is just a new feature being
> added to epoll, what bug does it "fix"?

The bug this fixes is that epoll today completely ignores the WF_SYNC
flag. The end result is the same, the wakee is woken, but the kernel has
several places where a synchronous wakeup is expected and with epoll
this isn't honored. However, it is honored with poll, select, recvmsg, etc.
Ultimately, epoll is inconsistent with other polling methods and this
inconsistency can lead to unexpected and surprising scheduling properties
based on the mechanism used in userspace.

For example, f467a6a664 ("pipe: fix and clarify pipe read wakeup logic")
highlighted the importance of sync wakeups for pipes, should GNU make
start using epoll we would see the same regression that resulted in this
fix from Linus.

>
> confused,
>
> greg k-h

Thanks,
Brian