Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1306855

Re: [PATCH 09/13] aio: add support for async openat()

Show all headers | View raw

On Mon, Jan 11, 2016 at 2:07 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
> Another blocking operation used by applications that want aio
> functionality is that of opening files that are not resident in memory.
> Using the thread based aio helper, add support for IOCB_CMD_OPENAT.

So I think this is ridiculously ugly.

AIO is a horrible ad-hoc design, with the main excuse being "other,
less gifted people, made that design, and we are implementing it for
compatibility because database people - who seldom have any shred of
taste - actually use it".

But AIO was always really really ugly.

Now you introduce the notion of doing almost arbitrary system calls
asynchronously in threads, but then you use that ass-backwards nasty
interface to do so.

Why?

If you want to do arbitrary asynchronous system calls, just *do* it.
But do _that_, not "let's extend this horrible interface in arbitrary
random ways one special system call at a time".

In other words, why is the interface not simply: "do arbitrary system
call X with arguments A, B, C, D asynchronously using a kernel
thread".

That's something that a lot of people might use. In fact, if they can
avoid the nasty AIO interface, maybe they'll even use it for things
like read() and write().

So I really think it would be a nice thing to allow some kind of
arbitrary "queue up asynchronous system call" model.

But I do not think the AIO model should be the model used for that,
even if I think there might be some shared infrastructure.

So I would seriously suggest:

 - how about we add a true "asynchronous system call" interface

 - make it be a list of system calls with a futex completion for each
list entry, so that you can easily wait for the end result that way.

 - maybe (and this is where it gets really iffy) you could even pass
in the result of one system call to the next, so that you can do
things like

       fd = openat(..)
       ret = read(fd, ..)

   asynchronously and then just wait for the read() to complete.

and let us *not* tie this to the aio interface.

In fact, if we do it well, we can go the other way, and try to
implement the nasty AIO interface on top of the generic "just do
things asynchronously".

And I actually think many of your kernel thread parts are good for a
generic implementation. That whole "AIO_THREAD_NEED_CRED" etc logic
all makes sense, although I do suspect you could just make it
unconditional. The cost of a few atomics shouldn't be excessive when
we're talking "use a thread to do op X".

What do you think? Do you think it might be possible to aim for a
generic "do system call asynchronously" model instead?

I'm adding Ingo the to cc, because I think Ingo had a "run this list
of system calls" patch at one point - in order to avoid system call
overhead. I don't think that was very interesting (because system call
overhead is seldom all that noticeable for any interesting system
calls), but with the "let's do the list asynchronously" addition it
might be much more intriguing. Ingo, do I remember correctly that it
was you? I might be confused about who wrote that patch, and I can't
find it now.

               Linus

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 00/13] aio: thread (work queue) based aio and new aio functionality Benjamin LaHaise <bcrl@kvack.org> - 2016-01-11 23:10 +0100
  [PATCH 09/13] aio: add support for async openat() Benjamin LaHaise <bcrl@kvack.org> - 2016-01-11 23:10 +0100
    Re: [PATCH 09/13] aio: add support for async openat() Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-12 01:30 +0100
      Re: [PATCH 09/13] aio: add support for async openat() Benjamin LaHaise <bcrl@kvack.org> - 2016-01-12 02:20 +0100
      Re: [PATCH 09/13] aio: add support for async openat() Chris Mason <clm@fb.com> - 2016-01-12 02:50 +0100
      Re: [PATCH 09/13] aio: add support for async openat() Ingo Molnar <mingo@kernel.org> - 2016-01-12 11:00 +0100
  [PATCH 04/13] signals: add and use aio_get_task() to direct signals sent via io_send_sig() Benjamin LaHaise <bcrl@kvack.org> - 2016-01-11 23:10 +0100
  [PATCH 10/13] aio: add async unlinkat functionality Benjamin LaHaise <bcrl@kvack.org> - 2016-01-11 23:10 +0100
  [PATCH 07/13] aio: enabled thread based async fsync Benjamin LaHaise <bcrl@kvack.org> - 2016-01-11 23:10 +0100
    Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-12 02:20 +0100
      Re: [PATCH 07/13] aio: enabled thread based async fsync Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-12 02:30 +0100
        Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-12 03:30 +0100
          Re: [PATCH 07/13] aio: enabled thread based async fsync Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-12 03:40 +0100
            Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-12 04:40 +0100
              Re: [PATCH 07/13] aio: enabled thread based async fsync Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-12 05:10 +0100
                Re: [PATCH 07/13] aio: enabled thread based async fsync Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-12 05:50 +0100
                Re: [PATCH 07/13] aio: enabled thread based async fsync Benjamin LaHaise <bcrl@kvack.org> - 2016-01-13 00:00 +0100
                Re: [PATCH 07/13] aio: enabled thread based async fsync Andy Lutomirski <luto@amacapital.net> - 2016-01-13 00:00 +0100
        Re: [PATCH 07/13] aio: enabled thread based async fsync Paolo Bonzini <pbonzini@redhat.com> - 2016-01-14 10:30 +0100
      Re: [PATCH 07/13] aio: enabled thread based async fsync Benjamin LaHaise <bcrl@kvack.org> - 2016-01-12 02:40 +0100
  [PATCH 06/13] aio: add queue_work() based threaded aio support Benjamin LaHaise <bcrl@kvack.org> - 2016-01-11 23:20 +0100

csiph-web