On 02/10/2018 01:10 AM, Dave Chinner wrote:

On Fri, Feb 09, 2018 at 02:11:58PM +0200, Avi Kivity wrote:

quoted

On 02/09/2018 12:11 AM, Dave Chinner wrote:

quoted

On Thu, Feb 08, 2018 at 10:24:11AM +0200, Avi Kivity wrote:

quoted

On 02/08/2018 01:33 AM, Dave Chinner wrote:

quoted

On Wed, Feb 07, 2018 at 07:20:17PM +0200, Avi Kivity wrote:

quoted

As usual, I'm having my lovely io_submit()s sleeping. This time some
detailed traces. 4.14.15.

[....]

quoted

quoted

quoted

Forcing the log, so sleeping with ILOCK taken.

Because it's trying to reallocate an extent that is pinned in the
log and is marked stale. i.e. we are reallocating a recently freed
metadata extent that hasn't been committed to disk yet. IOWs, it's
the metadata form of the "log force to clear a busy extent so we can
re-use it" condition....

There's nothing you can do to reliably avoid this - it's a sign that
you're running low on free space in an AG because it's recycling
recently freed space faster than the CIL is being committed to disk.

You could speed up background journal syncs to try to reduce the
async checkpoint latency that allows busy extents to build up
(/proc/sys/fs/xfs/xfssyncd_centisecs) but that also impacts on
journal overhead and IO latency, etc.

Perhaps xfs should auto-tune this variable.

That's not a fix. That's a nasty hack that attempts to hide the
underlying problem of selecting AGs and/or free space that requires
a log force to be used instead of finding other, un-encumbered
freespace present in the filesystem.

Isn't the underlying problem that you have a foreground process
depending on the progress of a background process?

At a very, very basic level.

quoted

i.e., no matter
how AG and free space selection improves, you can always find a
workload that consumes extents faster than they can be laundered?

Sure, but that doesn't mean we have to fall back to a synchronous
alogrithm to handle collisions. It's that synchronous behaviour that
is the root cause of the long lock stalls you are seeing.

Well, having that algorithm be asynchronous will be wonderful. But I 
imagine it will be a monstrous effort.

quoted

I'm not saying that free extent selection can't or shouldn't be
improved, just that it can never completely fix the problem on its
own.

Righto, if you say so.

After all, what do I know about the subject at hand? I'm just the
poor dumb guy

Just because you're an XFS expert, and even wrote the code at hand, 
doesn't mean I have nothing to contribute. If I'm wrong, it's enough to 
tell me that and why.

  who wrote the current busy extent list handling
algorithm years ago.  Perhaps you'd like to read the commit message
(below), because it explains these sycnhronous slow paths and why
they exist. I'll quote the part relevant to the discussion here,
though:

	    Ideally we should not reallocate busy extents. That is a
	    much more complex fix to the problem as it involves
	    direct intervention in the allocation btree searches in
	    many places. This is left to a future set of
	    modifications.

Thanks, that commit was interesting.

So, this future set of modifications is to have the extent allocator 
consult this rbtree and continue searching if locked?

Christoph has already mentioned in these busy extent blocking
threads that we need to do to avoid the synchronous blocking
problems you are seeing. We knew about this blocking problem more
than *15 years ago*, but we didn't need to solve it 8 years ago
because we it was not necessary to solve the "array overrun causes
log forces" problem people were reporting when they had heaps of
free space available in each AG.

Since then we haven't had a user report a workload that needed
anything more than what we currently do. You have a workload where
this is now important, and so we now need to revisit the issues we
laid out some 8 years ago and work from there.

I still think reducing the amount of outstanding busy extents is 
important. Modern disks write multiple GB/s, and big-data applications 
like to do large sequential writes and deletes, so high production rate 
of busy extents. Users also like high space utilization of their SSDs. 
It's easy to end up where most of your free extents are busy, leading to 
a hard time for your find-not-busy-free-extent algorithm.

quoted

quoted

quoted

If we could pass the iocb to file_update_time() then
xfs_vn_update_time, then it could perform a _nowait acquisition of
ILOCK. Otherwise, we can just check if ILOCK is acquirable in
xfs_file_aio_write_checks(). That's racy, but better than nothing.

Sure, but that's not the issue with such suggestions. I'm not about
to propose a patch to hack an iocb through generic infrastructure
that doesn't need an iocb. Doing stuff like that will only get me
shouted at because it's a massive layering violation and I should
(and do!) know better....

If updating the time is part of an asynchronous operation, then it
should fit with the rest of the async framework, no?

Perhaps a variant file_update_time_async() (which would be called by
file_update_time).

This doesn't fix the problem you've reported.

We've got to run a transaction to update timestamps. That means
memory allocation (which can block on IO) and transaction
reservations need to be made (which can block on IO) *before* we
even attempt to take the ilock.  That means there's a huge TOCTOU
race window in any "optimisitic" check we'd do here.

IOWs,  a non-blocking timestamp update API is still racy. It /might/
make blocking in timestamp updates less common, but it will not
improve the worst case long-tail latencies that are causing you
problems. And that means it doesn't fix your problem.

I'm aware it's racy. For me, even making it less common is an important 
improvement. I know I'll never get fully asynchronous behavior from the 
kernel, so I'm looking for the lowest hanging fruit rather than perfection.

I'm slightly helped by the fact that kernel memory allocations never 
block for me, as the kernel is never under memory pressure (my 
application allocates a fixed amount of anonymous memory and doesn't 
touch the page cache), but that's not the general case.

If we do have an async variant of file_update_time(), then the second 
check (for transaction reservation) can consult the NOWAIT flag and fail 
the write in that case with EAGAIN.

quoted

Well, I don't like it very much either. But I do need some fix.

We are well aware of that. And we are also well aware of just how
complex and difficult it is to solve properly.

Yeah.