[SRU][F][PATCH 0/1] xfs: Preallocated ioend transactions cause deadlock due to log buffer exhaustion

Tue Feb 14 05:50:21 UTC 2023

BugLink: https://bugs.launchpad.net/bugs/2007219

[Impact]

A deadlock exists in the XFS filesystem that occurs when the XFS log buffer
becomes completely exhausted.

XFS maintains a circular ring buffer for the log, and it is a fixed size. To be
able to create a transaction, you need to be able to reserve space on the log
buffer.

Certain ioend transactions, such as file append, can be preallocated for a
negligible performance gain. This takes up space in the log buffer, and these
preallocated ioends are placed on a workqueue, behind other ioends that are not
preallocated.

The deadlock occurs when the XFS log buffer is running low on space, and an
ioend append transaction comes in. It is preallocated, consuming nearly all of
the remaining XFS log buffer space, and is placed at the very end of the ioend
workqueue. The kernel then takes a ioend from the top of the ioend
workqeueue, creates a transaction, xfs_trans_alloc(), attempts to allocate space 
for it, xfs_trans_reserve(), xfs_log_reserve(), and then waits in a while loop
checking free space in the log, xlog_grant_head_check(), xlog_grant_head_wait().

Since there is no space, the thread sleeps with schedule(). This happens with
all ioend transactions, since the log is exhausted and I/O is not moving.

Since I/O never moves, the thread is never woken up, and we get hung task
timeouts, with system failure shortly afterward.

An example hung task timeout is:

INFO: task kworker/60:0:4002982 blocked for more than 360 seconds.
      Tainted: G           OE     5.4.0-137-generic #154-Ubuntu
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/60:0    D    0 4002982      2 0x90004080
Workqueue: xfs-conv/dm-3 xfs_end_io [xfs]
Call Trace:
 __schedule+0x2e3/0x740
 schedule+0x42/0xb0
 xlog_grant_head_wait+0xb9/0x1e0 [xfs]
 xlog_grant_head_check+0xde/0x100 [xfs]
 xfs_log_reserve+0xc9/0x1e0 [xfs]
 xfs_trans_reserve+0x17a/0x1e0 [xfs]
 xfs_trans_alloc+0xda/0x170 [xfs]
 xfs_iomap_write_unwritten+0x128/0x2f0 [xfs]
 xfs_end_ioend+0x15b/0x1b0 [xfs]
 xfs_end_io+0xb1/0xe0 [xfs]
 process_one_work+0x1eb/0x3b0
 worker_thread+0x4d/0x400
 kthread+0x104/0x140
 ? process_one_work+0x3b0/0x3b0
 ? kthread_park+0x90/0x90
 ret_from_fork+0x1f/0x40

There is no known workaround, other than to have a larger log buffer at
filesystem creation, but even then, it only buys you time until you get high
enough load to exhaust the log buffer.

[Fix]

This was fixed in 5.13-rc1 by the following patch:

commit 7cd3099f4925d7c15887d1940ebd65acd66100f5
Author: Brian Foster <bfoster at redhat.com>
Date:   Fri Apr 9 10:27:43 2021 -0700
Subject: xfs: drop submit side trans alloc for append ioends
Link: https://github.com/torvalds/linux/commit/7cd3099f4925d7c15887d1940ebd65acd66100f5

The patch more or less removes all preallocated ioend transactions, and instead,
when ioend appends are needed, they go to the standard workqueue like any other
ioend, where transactions are allocated when they reach the top of the 
workqueue. 

The patch required some backporting for Focal. The changes to the patch itself
is minimal and should be straightforward to read, however, the changes to the
XFS ioend subsystem between 5.4 and 5.13-rc1 were quite extreme, with a lot of
refactoring taking place over very many commits.

Additionally, the patch was part of a five part series, the first, fixes the
deadlock, and the rest remove all the code to do with transaction preallocation.

It is safe to leave the rest of the code in place. It will become dead code, but
it will not be reachable, and not cause any risk of regression, due to 
ioend->io_append_trans always being NULL, and not entering into any of the if
statements.

[Testcase]

There is currently no known testcase for this issue. The issue has only been
seen in a customer production environment, running under heavy load. The issue
has not been seen in a customer test environment, only production.

The production workload is a busy Kubernetes cluster running containers and VMs,
with the hosts's filesystem being broken into separate mountpoints over several
partitions, all XFS.

The kubernetes containers are all backed from a large 4TB disk which is XFS.

A test kernel is available in the following ppa:

https://launchpad.net/~mruffell/+archive/ubuntu/sf353709-test

This test kernel has been deployed to several production hosts, and the deadlock
no longer occurs.

[Where problems can occur]

We are changing how certain ioend transactions take place in the XFS filesystem.
If a regression were to occur, it would impact all XFS users. Users would have
to downgrade their kernel, as there are no workarounds for enabling or disabling
the behaviour change.

The overall risk of the change should be low. ioend append transactions would
still be processed in nearly the same way, still being placed at the end of the
ioend workqueue like any other transaction, with the only change being it is
allocated at the time the transaction is created, at the top of the workqueue
when it is processed, and not preallocated when the ioend is first submitted.

There will be a very minor performance penalty, but it wouldn't be measurable
in any tangible workload.

I have run xfstests for the xfs/* subset against the released 5.4.0-137-generic
and the test 5.4.0-137-generic test kernel with the patch included. The test
kernel had identical results, it will likely not cause any regressions.

There is some additional risk leaving the dead code in place, but I have read
the code and the commits to remove the dead code, and I came to the conclusion
it is less risky to leave it in place, than to backport the refactor commits.

[Other Info]

The XFS ioend subsystem refactor took place in the following commits:

commit 598ecfbaa742aca0dcdbbea25681406f95cc0b63
From: Christoph Hellwig <hch at lst.de>
Date: Thu, 17 Oct 2019 13:12:15 -0700
Subject: iomap: lift the xfs writeback code to iomap
Link: https://github.com/torvalds/linux/commit/598ecfbaa742aca0dcdbbea25681406f95cc0b63

commit 9e91c5728cab3d0aa3197d009c3d63e147914e77
From: Christoph Hellwig <hch at lst.de>
Date: Thu, 17 Oct 2019 13:12:13 -0700
Subject: iomap: lift common tracing code from xfs to iomap
Link: https://github.com/torvalds/linux/commit/9e91c5728cab3d0aa3197d009c3d63e147914e77

commit 433dad94ec5d6b90385b56a8bc8718dd9542b289
From: Christoph Hellwig <hch at lst.de>
Date: Thu, 17 Oct 2019 13:12:07 -0700
Subject: xfs: refactor the ioend merging code
Link: https://github.com/torvalds/linux/commit/433dad94ec5d6b90385b56a8bc8718dd9542b289

ioend->io_append_trans was renamed to ioend->io_private in the following commit:

commit 5653017bc44e54baa299f3523f160c23ac0628fd
From: Christoph Hellwig <hch at lst.de>
Date: Thu, 17 Oct 2019 13:12:09 -0700
Subject: xfs: turn io_append_trans into an io_private void pointer
Link: https://github.com/torvalds/linux/commit/5653017bc44e54baa299f3523f160c23ac0628fd

The full five part preallocated ioend deadlock patch series is:

commit 7cd3099f4925d7c15887d1940ebd65acd66100f5
Author: Brian Foster <bfoster at redhat.com>
Date:   Fri Apr 9 10:27:43 2021 -0700
Subject: xfs: drop submit side trans alloc for append ioends
Link: https://github.com/torvalds/linux/commit/7cd3099f4925d7c15887d1940ebd65acd66100f5

commit 7adb8f14e134d5f885d47c4ccd620836235f0b7f
Author: Brian Foster <bfoster at redhat.com>
Date:   Fri Apr 9 10:27:55 2021 -0700
Subject: xfs: open code ioend needs workqueue helper
Link: https://github.com/torvalds/linux/commit/7adb8f14e134d5f885d47c4ccd620836235f0b7f

commit 044c6449f18f174ba8d86640936add3fc7582e49
Author: Brian Foster <bfoster at redhat.com>
Date:   Fri Apr 9 10:27:55 2021 -0700
Subject: xfs: drop unused ioend private merge and setfilesize code
Link: https://github.com/torvalds/linux/commit/044c6449f18f174ba8d86640936add3fc7582e49

commit e7a3d7e792a5ad50583a2e6c35e72bd2ca6096f4
Author: Brian Foster <bfoster at redhat.com>
Date:   Fri Apr 9 10:27:56 2021 -0700
Subject: xfs: drop unnecessary setfilesize helper
Link: https://github.com/torvalds/linux/commit/e7a3d7e792a5ad50583a2e6c35e72bd2ca6096f4

commit 6e552494fb90acae005d74ce6a2ee102d965184b
Author: Brian Foster <bfoster at redhat.com>
Date:   Tue May 4 08:54:29 2021 -0700
Subject: iomap: remove unused private field from ioend
Link: https://github.com/torvalds/linux/commit/6e552494fb90acae005d74ce6a2ee102d965184b

As you can see, the four latter commits are not necessary. They simply remove
dead code, which has no harm being left in place. They also do not backport
at all, not without the ALL of the significant refactor commits.

Hence, we only take "xfs: drop submit side trans alloc for append ioends" only,
as it is the only needed commit to solve the problem.

Brian Foster (1):
  xfs: drop submit side trans alloc for append ioends

 fs/xfs/xfs_aops.c | 43 +++----------------------------------------
 1 file changed, 3 insertions(+), 40 deletions(-)

-- 
2.37.2