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


Groups > linux.kernel > #1310449 > unrolled thread

Re: [PATCH 07/13] aio: enabled thread based async fsync

Started byBenjamin LaHaise <bcrl@kvack.org>
First post2016-01-15 21:30 +0100
Last post2016-01-22 16:50 +0100
Articles 14 — 5 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 07/13] aio: enabled thread based async fsync Benjamin LaHaise <bcrl@kvack.org> - 2016-01-15 21:30 +0100
    Re: [PATCH 07/13] aio: enabled thread based async fsync Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-20 05:10 +0100
      Re: [PATCH 07/13] aio: enabled thread based async fsync Theodore Ts'o <tytso@mit.edu> - 2016-01-20 06:10 +0100
      Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-20 21:10 +0100
        Re: [PATCH 07/13] aio: enabled thread based async fsync Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-20 21:30 +0100
          Re: [PATCH 07/13] aio: enabled thread based async fsync Benjamin LaHaise <bcrl@kvack.org> - 2016-01-20 21:50 +0100
            Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-20 22:50 +0100
              Re: [PATCH 07/13] aio: enabled thread based async fsync Benjamin LaHaise <bcrl@kvack.org> - 2016-01-20 23:00 +0100
                Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-23 05:30 +0100
                  Re: [PATCH 07/13] aio: enabled thread based async fsync Benjamin LaHaise <bcrl@kvack.org> - 2016-01-23 06:00 +0100
                    Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-23 23:30 +0100
              Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-23 05:50 +0100
          Re: [PATCH 07/13] aio: enabled thread based async fsync Dave Chinner <david@fromorbit.com> - 2016-01-20 23:00 +0100
      Re: [PATCH 07/13] aio: enabled thread based async fsync Andres Freund <andres@anarazel.de> - 2016-01-22 16:50 +0100

#1310449 — Re: [PATCH 07/13] aio: enabled thread based async fsync

FromBenjamin LaHaise <bcrl@kvack.org>
Date2016-01-15 21:30 +0100
SubjectRe: [PATCH 07/13] aio: enabled thread based async fsync
Message-ID<qRjaq-2FM-11@gated-at.bofh.it>
On Mon, Jan 11, 2016 at 08:48:23PM -0800, Linus Torvalds wrote:
> On Mon, Jan 11, 2016 at 8:03 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > So my argument is really that I think it would be better to at least
> > look into maybe creating something less crapulent, and striving to
> > make it easy to make the old legacy interfaces be just wrappers around
> > a more capable model.
> 
> Hmm. Thinking more about this makes me worry about all the system call
> versioning and extra work done by libc.
> 
> At least glibc has traditionally decided to munge and extend on kernel
> system call interfaces, to the point where even fairly core data
> structures (like "struct stat") may not always look the same to the
> kernel as they do to user space.
> 
> So with that worry, I have to admit that maybe a limited interface -
> rather than allowing arbitrary generic async system calls - might have
> advantages. Less room for mismatches.
> 
> I'll have to think about this some more.

Any further thoughts on this after a few days worth of pondering?

		-ben

>                   Linus

-- 
"Thought is the essence of where you are now."

[toc] | [next] | [standalone]


#1312859

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-01-20 05:10 +0100
Message-ID<qSSfL-16M-5@gated-at.bofh.it>
In reply to#1310449
On Fri, Jan 15, 2016 at 12:21 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
>>
>> I'll have to think about this some more.
>
> Any further thoughts on this after a few days worth of pondering?

Sorry about the delay, with the merge window and me being sick for a
couple of days I didn't get around to this.

After thinking it over some more, I guess I'm ok with your approach.
The table-driven patch makes me a bit happier, and I guess not very
many people end up ever wanting to do async system calls anyway.

Are there other users outside of Solace? It would be good to get comments..

            Linus

[toc] | [prev] | [next] | [standalone]


#1312876

FromTheodore Ts'o <tytso@mit.edu>
Date2016-01-20 06:10 +0100
Message-ID<qSTbQ-1M2-19@gated-at.bofh.it>
In reply to#1312859
On Tue, Jan 19, 2016 at 07:59:35PM -0800, Linus Torvalds wrote:
> 
> After thinking it over some more, I guess I'm ok with your approach.
> The table-driven patch makes me a bit happier, and I guess not very
> many people end up ever wanting to do async system calls anyway.
> 
> Are there other users outside of Solace? It would be good to get comments..

For async I/O?  We're using it inside Google, for networking and for
storage I/O's.  We don't need async fsync/fdatasync, but we do need
very fast, low overhead I/O's.  To that end, we have some patches to
batch block layer completion handling, which Kent tried upstreaming a
few years back but which everyone thought was too ugly to live.

(It *was* ugly, but we had access to some very fast storage devices
where it really mattered.  With upcoming NVMe devices, that sort of
hardware should be available to more folks, so it's something that
I've been meaning to revisit from an upstreaming perspective,
especially if I can get my hands on some publically available hardware
for benchmarking purposes to demonstrate why it's useful, even if it
is ugly.)

The other thing which we have which is a bit more experimental is that
we've plumbed through the aio priority bits to the block layer, as
well as aio_cancel.  The idea for the latter is if you are are
interested in low latency access to a clustered file system, where
sometimes a read request can get stuck behind other I/O requests if a
server has a long queue of requests to service.  So the client for
which low latency is very important fires off the request to more than
one server, and as soon as it gets an answer it sends a "never mind"
message to the other server(s).

The code to do aio_cancellation in the block layer is fairly well
tested, and was in Kent's git trees, but never got formally pushed
upstream.  The code to push the cancellation request all the way to
the HDD (for those hard disks / storage devices that support I/O
cancellation) is even more experimental, and needs a lot of cleanup
before it could be sent for review (it was done by someone who isn't
used to upstream coding standards).

The reason why we haven't tried to pushed more of these changes
upsream has been lack of resources, and the fact that the AIO code
*is* ugly, which means extensions tend to make the code at the very
least, more complex.  Especially since some of the folks working on
it, such as Kent, were really worried about performance at all costs,
and Kerningham's "it's twice as hard to debug code as to write it"
comment really applies here.  And since very few people outside of
Google seem to use AIO, and even fewer seem eager to review or work on
AIO, and our team is quite small for the work we need to do, it just
hasn't risen to the top of the priority list.

Still, it's fair to say that if you are using Google Hangouts, or
Google Mail, or Google Docs, AIO is most definitely getting used to
process your queries.

As far as comments, aside from the "we really care about performance",
and "the code is scary complex and barely on the edge of being
maintainable", the other comment I'd make is libaio is pretty awful,
and so as a result a number (most?) of our AIO users have elected to
use the raw system call interfaces and are *not* using the libaio
abstractions --- which, as near as I can tell, don't really buy you
much anyway.  (Do we really need to keep code that provides backwards
compatibility with kernels over 10+ years old at this point?)

Cheers,

					- Ted

[toc] | [prev] | [next] | [standalone]


#1313486

FromDave Chinner <david@fromorbit.com>
Date2016-01-20 21:10 +0100
Message-ID<qT7eN-33h-1@gated-at.bofh.it>
In reply to#1312859
On Tue, Jan 19, 2016 at 07:59:35PM -0800, Linus Torvalds wrote:
> On Fri, Jan 15, 2016 at 12:21 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
> >>
> >> I'll have to think about this some more.
> >
> > Any further thoughts on this after a few days worth of pondering?
> 
> Sorry about the delay, with the merge window and me being sick for a
> couple of days I didn't get around to this.
> 
> After thinking it over some more, I guess I'm ok with your approach.
> The table-driven patch makes me a bit happier, and I guess not very
> many people end up ever wanting to do async system calls anyway.
> 
> Are there other users outside of Solace? It would be good to get comments..

I know of quite a few storage/db products that use AIO. The most
recent high profile project that have been reporting issues with AIO
on XFS is http://www.scylladb.com/. That project is architected
around non-blocking AIO for scalability reasons...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]


#1313507

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-01-20 21:30 +0100
Message-ID<qT7ya-3ae-27@gated-at.bofh.it>
In reply to#1313486
On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner <david@fromorbit.com> wrote:
>>
>> Are there other users outside of Solace? It would be good to get comments..
>
> I know of quite a few storage/db products that use AIO. The most
> recent high profile project that have been reporting issues with AIO
> on XFS is http://www.scylladb.com/. That project is architected
> around non-blocking AIO for scalability reasons...

I was more wondering about the new interfaces, making sure that the
feature set actually matches what people want to do..

That said, I also agree that it would be interesting to hear what the
performance impact is for existing performance-sensitive users. Could
we make that "aio_may_use_threads()" case be unconditional, making
things simpler?

          Linus

[toc] | [prev] | [next] | [standalone]


#1313518

FromBenjamin LaHaise <bcrl@kvack.org>
Date2016-01-20 21:50 +0100
Message-ID<qT7Rv-3iq-1@gated-at.bofh.it>
In reply to#1313507
On Wed, Jan 20, 2016 at 12:29:32PM -0800, Linus Torvalds wrote:
> On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner <david@fromorbit.com> wrote:
> >>
> >> Are there other users outside of Solace? It would be good to get comments..
> >
> > I know of quite a few storage/db products that use AIO. The most
> > recent high profile project that have been reporting issues with AIO
> > on XFS is http://www.scylladb.com/. That project is architected
> > around non-blocking AIO for scalability reasons...
> 
> I was more wondering about the new interfaces, making sure that the
> feature set actually matches what people want to do..

I suspect this will be an ongoing learning exercise as people start to use 
the new functionality and find gaps in terms of what is needed.  Certainly 
there is a bunch of stuff we need to add to cover the cases where disk i/o 
is required.  getdents() is one example, but the ABI issues we have with it 
are somewhat more complicated given the history associated with that 
interface.

> That said, I also agree that it would be interesting to hear what the
> performance impact is for existing performance-sensitive users. Could
> we make that "aio_may_use_threads()" case be unconditional, making
> things simpler?

Making it unconditional is a goal, but some work is required before that 
can be the case.  The O_DIRECT issue is one such matter -- it requires some 
changes to the filesystems to ensure that they adhere to the non-blocking 
nature of the new interface (ie taking i_mutex is a Bad Thing that users 
really do not want to be exposed to; if taking it blocks, the code should 
punt to a helper thread).  Additional auditing of some of the read/write 
implementations is also required, which will likely need some minor changes 
in things like sysfs and other weird functionality we have.  Having the 
flag reflects that while the functionality is useful, not all of the bugs 
have been worked out yet.

What's the desired approach to merge these changes?  Does it make sense 
to merge what is ready now and prepare the next round of changes for 4.6?  
Or is it more important to grow things to a more complete state before 
merging?

Regards,

		-ben

>           Linus

-- 
"Thought is the essence of where you are now."

[toc] | [prev] | [next] | [standalone]


#1313538

FromDave Chinner <david@fromorbit.com>
Date2016-01-20 22:50 +0100
Message-ID<qT8Nz-3TE-13@gated-at.bofh.it>
In reply to#1313518
On Wed, Jan 20, 2016 at 03:44:49PM -0500, Benjamin LaHaise wrote:
> On Wed, Jan 20, 2016 at 12:29:32PM -0800, Linus Torvalds wrote:
> > On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner <david@fromorbit.com> wrote:
> > >>
> > >> Are there other users outside of Solace? It would be good to get comments..
> > >
> > > I know of quite a few storage/db products that use AIO. The most
> > > recent high profile project that have been reporting issues with AIO
> > > on XFS is http://www.scylladb.com/. That project is architected
> > > around non-blocking AIO for scalability reasons...
> > 
> > I was more wondering about the new interfaces, making sure that the
> > feature set actually matches what people want to do..
> 
> I suspect this will be an ongoing learning exercise as people start to use 
> the new functionality and find gaps in terms of what is needed.  Certainly 
> there is a bunch of stuff we need to add to cover the cases where disk i/o 
> is required.  getdents() is one example, but the ABI issues we have with it 
> are somewhat more complicated given the history associated with that 
> interface.
> 
> > That said, I also agree that it would be interesting to hear what the
> > performance impact is for existing performance-sensitive users. Could
> > we make that "aio_may_use_threads()" case be unconditional, making
> > things simpler?
> 
> Making it unconditional is a goal, but some work is required before that 
> can be the case.  The O_DIRECT issue is one such matter -- it requires some 
> changes to the filesystems to ensure that they adhere to the non-blocking 
> nature of the new interface (ie taking i_mutex is a Bad Thing that users 
> really do not want to be exposed to; if taking it blocks, the code should 
> punt to a helper thread).

Filesystems *must take locks* in the IO path. We have to serialise
against truncate and other operations at some point in the IO path
(e.g. block mapping vs concurrent allocation and/or removal), and
that can only be done sanely with sleeping locks.  There is no way
of knowing in advance if we are going to block, and so either we
always use threads for IO submission or we accept that occasionally
the AIO submission will block.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]


#1313542

FromBenjamin LaHaise <bcrl@kvack.org>
Date2016-01-20 23:00 +0100
Message-ID<qT8Xg-3WS-9@gated-at.bofh.it>
In reply to#1313538
On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
> Filesystems *must take locks* in the IO path. We have to serialise
> against truncate and other operations at some point in the IO path
> (e.g. block mapping vs concurrent allocation and/or removal), and
> that can only be done sanely with sleeping locks.  There is no way
> of knowing in advance if we are going to block, and so either we
> always use threads for IO submission or we accept that occasionally
> the AIO submission will block.

I never said we don't take locks.  Still, we can be more intelligent 
about when and where we do so.  With the nonblocking pread() and pwrite() 
changes being proposed elsewhere, we can do the part of the I/O that 
doesn't block in the submitter, which is a huge win when possible.

As it stands today, *every* buffered write takes i_mutex immediately 
on entering ->write().  That one issue alone accounts for a nearly 10x 
performance difference between an O_SYNC write and an O_DIRECT write, 
and using O_SYNC writes is a legitimate use-case for users who want 
caching of data by the kernel (duplicating that functionality is a huge 
amount of work for an application, plus if you want the cache to be 
persistent between runs of an app, you have to get the kernel to do it).

		-ben

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

-- 
"Thought is the essence of where you are now."

[toc] | [prev] | [next] | [standalone]


#1315516

FromDave Chinner <david@fromorbit.com>
Date2016-01-23 05:30 +0100
Message-ID<qTXZL-5LO-7@gated-at.bofh.it>
In reply to#1313542
On Wed, Jan 20, 2016 at 04:56:30PM -0500, Benjamin LaHaise wrote:
> On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
> > Filesystems *must take locks* in the IO path. We have to serialise
> > against truncate and other operations at some point in the IO path
> > (e.g. block mapping vs concurrent allocation and/or removal), and
> > that can only be done sanely with sleeping locks.  There is no way
> > of knowing in advance if we are going to block, and so either we
> > always use threads for IO submission or we accept that occasionally
> > the AIO submission will block.
> 
> I never said we don't take locks.  Still, we can be more intelligent 
> about when and where we do so.  With the nonblocking pread() and pwrite() 
> changes being proposed elsewhere, we can do the part of the I/O that 
> doesn't block in the submitter, which is a huge win when possible.
> 
> As it stands today, *every* buffered write takes i_mutex immediately 
> on entering ->write().  That one issue alone accounts for a nearly 10x 
> performance difference between an O_SYNC write and an O_DIRECT write, 

Yes, that locking is for correct behaviour, not for performance
reasons.  The i_mutex is providing the required semantics for POSIX
write(2) functionality - writes must serialise against other reads
and writes so that they are completed atomically w.r.t. other IO.
i.e. writes to the same offset must not interleave, not should reads
be able to see partial data from a write in progress.

Direct IO does not conform to POSIX concurrency standards, so we
don't have to serialise concurrent IO against each other.

> and using O_SYNC writes is a legitimate use-case for users who want 
> caching of data by the kernel (duplicating that functionality is a huge 
> amount of work for an application, plus if you want the cache to be 
> persistent between runs of an app, you have to get the kernel to do it).

Yes, but you take what you get given. Buffered IO sucks in many ways;
this is just one of them.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]


#1315520

FromBenjamin LaHaise <bcrl@kvack.org>
Date2016-01-23 06:00 +0100
Message-ID<qTYsO-5WT-1@gated-at.bofh.it>
In reply to#1315516
On Sat, Jan 23, 2016 at 03:24:49PM +1100, Dave Chinner wrote:
> On Wed, Jan 20, 2016 at 04:56:30PM -0500, Benjamin LaHaise wrote:
> > On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
> > > Filesystems *must take locks* in the IO path. We have to serialise
> > > against truncate and other operations at some point in the IO path
> > > (e.g. block mapping vs concurrent allocation and/or removal), and
> > > that can only be done sanely with sleeping locks.  There is no way
> > > of knowing in advance if we are going to block, and so either we
> > > always use threads for IO submission or we accept that occasionally
> > > the AIO submission will block.
> > 
> > I never said we don't take locks.  Still, we can be more intelligent 
> > about when and where we do so.  With the nonblocking pread() and pwrite() 
> > changes being proposed elsewhere, we can do the part of the I/O that 
> > doesn't block in the submitter, which is a huge win when possible.
> > 
> > As it stands today, *every* buffered write takes i_mutex immediately 
> > on entering ->write().  That one issue alone accounts for a nearly 10x 
> > performance difference between an O_SYNC write and an O_DIRECT write, 
> 
> Yes, that locking is for correct behaviour, not for performance
> reasons.  The i_mutex is providing the required semantics for POSIX
> write(2) functionality - writes must serialise against other reads
> and writes so that they are completed atomically w.r.t. other IO.
> i.e. writes to the same offset must not interleave, not should reads
> be able to see partial data from a write in progress.

No, the locks are not *required* for POSIX semantics, they are a legacy
of how Linux filesystem code has been implemented and how we ensure the
necessary internal consistency needed inside our filesystems is
provided.  There are other ways to achieve the required semantics that
do not involve a single giant lock for the entire file/inode.  And no, I
am not saying that doing this is simple or easy to do.

		-ben

> Direct IO does not conform to POSIX concurrency standards, so we
> don't have to serialise concurrent IO against each other.
> 
> > and using O_SYNC writes is a legitimate use-case for users who want 
> > caching of data by the kernel (duplicating that functionality is a huge 
> > amount of work for an application, plus if you want the cache to be 
> > persistent between runs of an app, you have to get the kernel to do it).
> 
> Yes, but you take what you get given. Buffered IO sucks in many ways;
> this is just one of them.
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

-- 
"Thought is the essence of where you are now."

[toc] | [prev] | [next] | [standalone]


#1315769

FromDave Chinner <david@fromorbit.com>
Date2016-01-23 23:30 +0100
Message-ID<qUeQV-3GP-5@gated-at.bofh.it>
In reply to#1315520
On Fri, Jan 22, 2016 at 11:50:24PM -0500, Benjamin LaHaise wrote:
> On Sat, Jan 23, 2016 at 03:24:49PM +1100, Dave Chinner wrote:
> > On Wed, Jan 20, 2016 at 04:56:30PM -0500, Benjamin LaHaise wrote:
> > > On Thu, Jan 21, 2016 at 08:45:46AM +1100, Dave Chinner wrote:
> > > > Filesystems *must take locks* in the IO path. We have to serialise
> > > > against truncate and other operations at some point in the IO path
> > > > (e.g. block mapping vs concurrent allocation and/or removal), and
> > > > that can only be done sanely with sleeping locks.  There is no way
> > > > of knowing in advance if we are going to block, and so either we
> > > > always use threads for IO submission or we accept that occasionally
> > > > the AIO submission will block.
> > > 
> > > I never said we don't take locks.  Still, we can be more intelligent 
> > > about when and where we do so.  With the nonblocking pread() and pwrite() 
> > > changes being proposed elsewhere, we can do the part of the I/O that 
> > > doesn't block in the submitter, which is a huge win when possible.
> > > 
> > > As it stands today, *every* buffered write takes i_mutex immediately 
> > > on entering ->write().  That one issue alone accounts for a nearly 10x 
> > > performance difference between an O_SYNC write and an O_DIRECT write, 
> > 
> > Yes, that locking is for correct behaviour, not for performance
> > reasons.  The i_mutex is providing the required semantics for POSIX
> > write(2) functionality - writes must serialise against other reads
> > and writes so that they are completed atomically w.r.t. other IO.
> > i.e. writes to the same offset must not interleave, not should reads
> > be able to see partial data from a write in progress.
> 
> No, the locks are not *required* for POSIX semantics, they are a legacy
> of how Linux filesystem code has been implemented and how we ensure the
> necessary internal consistency needed inside our filesystems is
> provided.

That may be the case, but I really don't see how you can provide
such required functionality without some kind of exclusion barrier
in place. No matter how you implement that exclusion, it can be seen
effectively as a lock.

Even if the filesystem doesn't use the i_mutex for exclusion to the
page cache, it has to use some kind of lock as that IO still needs
to be serialised against any truncate, hole punch or other extent
manipulation that is currently in progress on the inode...

> There are other ways to achieve the required semantics that
> do not involve a single giant lock for the entire file/inode.

Most performant filesystems don't have a "single giant lock"
anymore. The problem is that the VFS expects the i_mutex to be held
for certain operations in the IO path and the VFS lock order
heirarchy makes it impossible to do anything but "get i_mutex
first".  That's the problem that needs to be solved - the VFS
enforces the "one giant lock" model, even when underlying
filesystems do not require it.

i.e. we could quite happily remove the i_mutex completely from the XFS
buffered IO path without breaking anything, but we can't because
that results in the VFS throwing warnings that we don't hold the
i_mutex (e.g like when removing the SUID bits on write). So there's
lots of VFS functionality that needs to be turned on it's head
before the i_mutex can be removed from the IO path.

> And no, I
> am not saying that doing this is simple or easy to do.

Sure. That's always been the problem. Even when a split IO/metadata
locking strategy like what XFS uses (and other modern filesystems
are moving to internally) is suggested as a model for solving
these problems, the usual response instant dismissal with
"no way, that's unworkable" and so nothing ever changes...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]


#1315519

FromDave Chinner <david@fromorbit.com>
Date2016-01-23 05:50 +0100
Message-ID<qTYj8-5TC-13@gated-at.bofh.it>
In reply to#1313538
On Wed, Jan 20, 2016 at 03:07:26PM -0800, Linus Torvalds wrote:
> On Jan 20, 2016 1:46 PM, "Dave Chinner" <david@fromorbit.com> wrote:
> > >
> > > > That said, I also agree that it would be interesting to hear what the
> > > > performance impact is for existing performance-sensitive users. Could
> > > > we make that "aio_may_use_threads()" case be unconditional, making
> > > > things simpler?
> > >
> > > Making it unconditional is a goal, but some work is required before that
> > > can be the case.  The O_DIRECT issue is one such matter -- it requires
> some
> > > changes to the filesystems to ensure that they adhere to the
> non-blocking
> > > nature of the new interface (ie taking i_mutex is a Bad Thing that users
> > > really do not want to be exposed to; if taking it blocks, the code
> should
> > > punt to a helper thread).
> >
> > Filesystems *must take locks* in the IO path.
> 
> I agree.
> 
> I also would prefer to make the aio code have as little interaction and
> magic flags with the filesystem code as humanly possible.
> 
> I wonder if we could make the rough rule be that the only synchronous case
> the aio code ever has is more or less entirely in the generic vfs caches?
> IOW, could we possibly aim to make the rule be that if we call down to the
> filesystem layer, we do that within a thread?

We have to go through the filesystem layer locking even on page
cache hits, and even if we get into the page cache copy-in/copy-out
code we can still get stuck on things like page locks and page
faults. Even if hte pages are cached, we can still get caught on
deeper filesystem locks for block mapping. e.g. read from a hole,
get zeros back, page cache is populated. Write data into range,
fetch page, realise it's unmapped, need to do block/delayed
allocation which requires filesystem locks and potentially
transactions and IO....

> We could do things like that for the name loopkup for openat() too, where
> we could handle the successful RCU loopkup synchronously, but then if we
> fall out of RCU mode we'd do the thread.

We'd have to do quite a bit of work to unwind back out to the AIO
layer before we can dispatch the open operation again in a thread,
wouldn't we?

So I'm not convinced that conditional thread dispatch makes sense. I
think the simplest thing to do is make all AIO use threads/
workqueues by default, and if the application is smart enough to
only do things that minimise blocking they can turn off the threaded
dispatch and get the same behaviour they get now.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]


#1313541

FromDave Chinner <david@fromorbit.com>
Date2016-01-20 23:00 +0100
Message-ID<qT8Xg-3WS-11@gated-at.bofh.it>
In reply to#1313507
On Wed, Jan 20, 2016 at 12:29:32PM -0800, Linus Torvalds wrote:
> On Wed, Jan 20, 2016 at 11:59 AM, Dave Chinner <david@fromorbit.com> wrote:
> >>
> >> Are there other users outside of Solace? It would be good to get comments..
> >
> > I know of quite a few storage/db products that use AIO. The most
> > recent high profile project that have been reporting issues with AIO
> > on XFS is http://www.scylladb.com/. That project is architected
> > around non-blocking AIO for scalability reasons...
> 
> I was more wondering about the new interfaces, making sure that the
> feature set actually matches what people want to do..

Well, they have mentioned that openat() can block, as will the first
operation after open that requires reading the file extent map from
disk. There are some ways of hacking around this (e.g. running
FIEMAP with a zero extent count or ext4's special extent prefetch
ioctl in a separate thread to prefetch the extent list into memory
before IO is required) so I suspect we may actually need some
interfaces that don't current exist at all....

> That said, I also agree that it would be interesting to hear what the
> performance impact is for existing performance-sensitive users. Could
> we make that "aio_may_use_threads()" case be unconditional, making
> things simpler?

That would make things a lot simpler in the kernel and AIO
submission a lot more predictable/deterministic for userspace. I'd
suggest that, at minimum, it should be the default behaviour...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]


#1315083

FromAndres Freund <andres@anarazel.de>
Date2016-01-22 16:50 +0100
Message-ID<qTM8i-5YV-21@gated-at.bofh.it>
In reply to#1312859
On 2016-01-19 19:59:35 -0800, Linus Torvalds wrote:
> Are there other users outside of Solace? It would be good to get comments..

PostgreSQL is a potential user of async fdatasync, fsync,
sync_file_range and potentially readahead, write, read. First tests with
Dave's async fsync/fsync_range are positive, so are the results with a
self-hacked async sync_file_range (although I'm kinda thinking that it
shouldn't really require to be used asynchronously).

I rather doubt openat, unlink et al are going to be interesting for
*us*, the requires structural changes would be too bit. But obviously
that doesn't mean anything for others.

Andres

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web