Skip to content

Commit b822ea1

Browse files
author
Darrick J. Wong
committed
xfs: always free xattri_leaf_bp when cancelling a deferred op
While running the following fstest with logged xattrs DISabled, I noticed the following: # FSSTRESS_AVOID="-z -f unlink=1 -f rmdir=1 -f creat=2 -f mkdir=2 -f getfattr=3 -f listfattr=3 -f attr_remove=4 -f removefattr=4 -f setfattr=20 -f attr_set=60" ./check generic/475 INFO: task u9:1:40 blocked for more than 61 seconds. Tainted: G O 5.19.0-rc2-djwx #rc2 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:u9:1 state:D stack:12872 pid: 40 ppid: 2 flags:0x00004000 Workqueue: xfs-cil/dm-0 xlog_cil_push_work [xfs] Call Trace: <TASK> __schedule+0x2db/0x1110 schedule+0x58/0xc0 schedule_timeout+0x115/0x160 __down_common+0x126/0x210 down+0x54/0x70 xfs_buf_lock+0x2d/0xe0 [xfs 0532c1cb1d67dd81d15cb79ac6e415c8dec58f73] xfs_buf_item_unpin+0x227/0x3a0 [xfs 0532c1cb1d67dd81d15cb79ac6e415c8dec58f73] xfs_trans_committed_bulk+0x18e/0x320 [xfs 0532c1cb1d67dd81d15cb79ac6e415c8dec58f73] xlog_cil_committed+0x2ea/0x360 [xfs 0532c1cb1d67dd81d15cb79ac6e415c8dec58f73] xlog_cil_push_work+0x60f/0x690 [xfs 0532c1cb1d67dd81d15cb79ac6e415c8dec58f73] process_one_work+0x1df/0x3c0 worker_thread+0x53/0x3b0 kthread+0xea/0x110 ret_from_fork+0x1f/0x30 </TASK> This appears to be the result of shortform_to_leaf creating a new leaf buffer as part of adding an xattr to a file. The new leaf buffer is held and attached to the xfs_attr_intent structure, but then the filesystem shuts down. Instead of the usual path (which adds the attr to the held leaf buffer which releases the hold), we instead cancel the entire deferred operation. Unfortunately, xfs_attr_cancel_item doesn't release any attached leaf buffers, so we leak the locked buffer. The CIL cannot do anything about that, and hangs. Fix this by teaching it to release leaf buffers, and make XFS a little more careful about not leaving a dangling reference. The prologue of xfs_attri_item_recover is (in this author's opinion) a little hard to figure out, so I'll clean that up in the next patch. Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Dave Chinner <[email protected]>
1 parent 82af880 commit b822ea1

File tree

1 file changed

+19
-1
lines changed

1 file changed

+19
-1
lines changed

fs/xfs/xfs_attr_item.c

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,8 @@ static inline void
455455
xfs_attr_free_item(
456456
struct xfs_attr_intent *attr)
457457
{
458+
ASSERT(attr->xattri_leaf_bp == NULL);
459+
458460
if (attr->xattri_da_state)
459461
xfs_da_state_free(attr->xattri_da_state);
460462
xfs_attri_log_nameval_put(attr->xattri_nameval);
@@ -509,6 +511,10 @@ xfs_attr_cancel_item(
509511
struct xfs_attr_intent *attr;
510512

511513
attr = container_of(item, struct xfs_attr_intent, xattri_list);
514+
if (attr->xattri_leaf_bp) {
515+
xfs_buf_relse(attr->xattri_leaf_bp);
516+
attr->xattri_leaf_bp = NULL;
517+
}
512518
xfs_attr_free_item(attr);
513519
}
514520

@@ -670,9 +676,21 @@ xfs_attri_item_recover(
670676
error = xfs_defer_ops_capture_and_commit(tp, capture_list);
671677

672678
out_unlock:
673-
if (attr->xattri_leaf_bp)
679+
if (attr->xattri_leaf_bp) {
674680
xfs_buf_relse(attr->xattri_leaf_bp);
675681

682+
/*
683+
* If there's more work to do to complete the attr intent, the
684+
* defer capture structure will have taken its own reference to
685+
* the attr leaf buffer and will give that to the continuation
686+
* transaction. The attr intent struct drives the continuation
687+
* work, so release our refcount on the attr leaf buffer but
688+
* retain the pointer in the intent structure.
689+
*/
690+
if (ret != -EAGAIN)
691+
attr->xattri_leaf_bp = NULL;
692+
}
693+
676694
xfs_iunlock(ip, XFS_ILOCK_EXCL);
677695
xfs_irele(ip);
678696
out:

0 commit comments

Comments
 (0)