[GIT PULL] aio poll fixes for 5.16-rc5

[GIT PULL] aio poll fixes for 5.16-rc5

[Eric Biggers]

The following changes since commit 0fcfb00b28c0b7884635dacf38e46d60bf3d4eb1:

  Linux 5.16-rc4 (2021-12-05 14:08:22 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git tags/aio-poll-for-linus

for you to fetch changes up to 4b3749865374899e115aa8c48681709b086fe6d3:

  aio: Fix incorrect usage of eventfd_signal_allowed() (2021-12-09 10:52:55 -0800)

----------------------------------------------------------------

Fix three bugs in aio poll, and one issue with POLLFREE more broadly:

  - aio poll didn't handle POLLFREE, causing a use-after-free.
  - aio poll could block while the file is ready.
  - aio poll called eventfd_signal() when it isn't allowed.
  - POLLFREE didn't handle multiple exclusive waiters correctly.

This has been tested with the libaio test suite, as well as with test
programs I wrote that reproduce the first two bugs.  I am sending this
pull request myself as no one seems to be maintaining this code.

----------------------------------------------------------------
Eric Biggers (5):
      wait: add wake_up_pollfree()
      binder: use wake_up_pollfree()
      signalfd: use wake_up_pollfree()
      aio: keep poll requests on waitqueue until completed
      aio: fix use-after-free due to missing POLLFREE handling

Xie Yongji (1):
      aio: Fix incorrect usage of eventfd_signal_allowed()

 drivers/android/binder.c        |  21 ++---
 fs/aio.c                        | 186 ++++++++++++++++++++++++++++++++--------
 fs/signalfd.c                   |  12 +--
 include/linux/wait.h            |  26 ++++++
 include/uapi/asm-generic/poll.h |   2 +-
 kernel/sched/wait.c             |   7 ++
 6 files changed, 196 insertions(+), 58 deletions(-)

Re: [GIT PULL] aio poll fixes for 5.16-rc5

[Linus Torvalds]

On Fri, Dec 10, 2021 at 10:33 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> This has been tested with the libaio test suite, as well as with test
> programs I wrote that reproduce the first two bugs.  I am sending this
> pull request myself as no one seems to be maintaining this code.

Pulled.

The "nobody really maintains or cares about epoll/aio" makes me wonder
if we should just remove the "if EXPERT" from the config options we
have on them, and start encouraging people to perhaps not even build
that code any more?

Because I'm sure we have users of it, but maybe they are few enough
that saying "don't enable this feature unless you need it" is the
right thing to do...

               Linus

Re: [GIT PULL] aio poll fixes for 5.16-rc5

[Eric Biggers]

On Fri, Dec 10, 2021 at 02:18:12PM -0800, Linus Torvalds wrote:
> On Fri, Dec 10, 2021 at 10:33 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > This has been tested with the libaio test suite, as well as with test
> > programs I wrote that reproduce the first two bugs.  I am sending this
> > pull request myself as no one seems to be maintaining this code.
> 
> Pulled.
> 
> The "nobody really maintains or cares about epoll/aio" makes me wonder
> if we should just remove the "if EXPERT" from the config options we
> have on them, and start encouraging people to perhaps not even build
> that code any more?
> 
> Because I'm sure we have users of it, but maybe they are few enough
> that saying "don't enable this feature unless you need it" is the
> right thing to do...

Isn't epoll more commonly used than aio?  Either way, removing 'if EXPERT' from
both would make sense, so that they aren't forced on just because someone didn't
set CONFIG_EXPERT.  I think that a lot of people have these options enabled but
don't need them.  Android used to be in that boat for CONFIG_AIO (i.e. it wasn't
needed but was sometimes enabled anyway, maybe due to !CONFIG_EXPERT).
Unfortunately Android has started depending on CONFIG_AIO, so it seems Android
will need to keep it set, but I think most other Linux systems don't need it.

- Eric

Re: [GIT PULL] aio poll fixes for 5.16-rc5

[Theodore Y. Ts'o]

On Fri, Dec 10, 2021 at 03:00:36PM -0800, Eric Biggers wrote:
> 
> Isn't epoll more commonly used than aio?  Either way, removing 'if EXPERT' from
> both would make sense, so that they aren't forced on just because someone didn't
> set CONFIG_EXPERT.  I think that a lot of people have these options enabled but
> don't need them.  Android used to be in that boat for CONFIG_AIO (i.e. it wasn't
> needed but was sometimes enabled anyway, maybe due to !CONFIG_EXPERT).
> Unfortunately Android has started depending on CONFIG_AIO, so it seems Android
> will need to keep it set, but I think most other Linux systems don't need it.

Mysql and Postgress both can use libaio, and I suspect many
distributions are compiling them with AIO enabled, since you can get
better performance with AIO.  Fio also uses AIO, and many fio recipes
that are trying to benchmark file systems or block devices use
AIO/DIO.

It's fair to say that the libaio programming interface is pretty
horrendo, and so very few application programmers will happy choosing
to use it.  But if you really care about storage performance, whether
you're implementing a database, or a cluster file system, it's likely
that you will find yourself deciding to try to use it.

The fact that so many storage-centric userspace programs use it
*desite* libaio's developer-hostile interface is a proof point of how
much AIO can help with performance.  Maybe over time we can get folks
to switch to io_uring, and we will eventually be able to get us to the
happy place where most Linux systems won't need CONFIG_AIO.  But that
day is not today.  :-/

So removing the dependency on CONFIG_EXPERT is probably a good idea,
at least for now.

Cheers,

						- Ted

Re: [GIT PULL] aio poll fixes for 5.16-rc5

[Christoph Hellwig]

On Fri, Dec 10, 2021 at 07:45:39PM -0500, Theodore Y. Ts'o wrote:
> distributions are compiling them with AIO enabled, since you can get
> better performance with AIO.  Fio also uses AIO, and many fio recipes
> that are trying to benchmark file systems or block devices use
> AIO/DIO.

As does qemu and many commercial databases.

Re: [GIT PULL] aio poll fixes for 5.16-rc5

[pr-tracker-bot]

The pull request you sent on Fri, 10 Dec 2021 10:32:55 -0800:

> https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git tags/aio-poll-for-linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/0d21e6684779493d90f3dee90d4457d5b4aed8ad

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html