xfs: remove the leftover xfs_{set,clear}_li_failed infrastructure
authorChristoph Hellwig <hch@lst.de>
Thu, 20 Mar 2025 07:52:13 +0000 (08:52 +0100)
committerCarlos Maiolino <cem@kernel.org>
Mon, 14 Apr 2025 08:24:30 +0000 (10:24 +0200)
Marking a log item as failed kept a buffer reference around for
resubmission of inode and dquote items.

For inode items commit 298f7bec503f3 ("xfs: pin inode backing buffer to
the inode log item") started pinning the inode item buffers
unconditionally and removed the need for this.  Later commit acc8f8628c37
("xfs: attach dquot buffer to dquot log item buffer") did the same for
dquot items but didn't fully clean up the xfs_clear_li_failed side
for them.  Stop adding the extra pin for dquot items and remove the
helpers.

This happens to fix a call to xfs_buf_free with the AIL lock held,
which would be incorrect for the unlikely case freeing the buffer
ends up calling vfree.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
fs/xfs/xfs_dquot.c
fs/xfs/xfs_inode_item.c
fs/xfs/xfs_trans_ail.c
fs/xfs/xfs_trans_priv.h

index edbc521870a10a1086098dd7bd105d510275a88b..b4e32f0860b7e68a97e8887f6d8ba38ad4ba48be 100644 (file)
@@ -1186,9 +1186,8 @@ xfs_qm_dqflush_done(
        if (test_bit(XFS_LI_IN_AIL, &lip->li_flags) &&
            (lip->li_lsn == qlip->qli_flush_lsn ||
             test_bit(XFS_LI_FAILED, &lip->li_flags))) {
-
                spin_lock(&ailp->ail_lock);
-               xfs_clear_li_failed(lip);
+               clear_bit(XFS_LI_FAILED, &lip->li_flags);
                if (lip->li_lsn == qlip->qli_flush_lsn) {
                        /* xfs_ail_update_finish() drops the AIL lock */
                        tail_lsn = xfs_ail_delete_one(ailp, lip);
index 40fc1bf900af90ac6425c036955c41b1b8285204..c6cb0b6b9e4605655b9b4de62d5e94c257f9c81b 100644 (file)
@@ -1089,13 +1089,7 @@ xfs_iflush_abort(
         * state. Whilst the inode is in the AIL, it should have a valid buffer
         * pointer for push operations to access - it is only safe to remove the
         * inode from the buffer once it has been removed from the AIL.
-        *
-        * We also clear the failed bit before removing the item from the AIL
-        * as xfs_trans_ail_delete()->xfs_clear_li_failed() will release buffer
-        * references the inode item owns and needs to hold until we've fully
-        * aborted the inode log item and detached it from the buffer.
         */
-       clear_bit(XFS_LI_FAILED, &iip->ili_item.li_flags);
        xfs_trans_ail_delete(&iip->ili_item, 0);
 
        /*
index 0fcb1828e598fbbe5ddeb88b2ff443da389bf0f6..85a649fec6acf652f33ab57006cb64c2858909f3 100644 (file)
@@ -909,10 +909,9 @@ xfs_trans_ail_delete(
                return;
        }
 
-       /* xfs_ail_update_finish() drops the AIL lock */
-       xfs_clear_li_failed(lip);
+       clear_bit(XFS_LI_FAILED, &lip->li_flags);
        tail_lsn = xfs_ail_delete_one(ailp, lip);
-       xfs_ail_update_finish(ailp, tail_lsn);
+       xfs_ail_update_finish(ailp, tail_lsn);  /* drops the AIL lock */
 }
 
 int
index bd841df93021ffe0aacb747449aa9131d3c4596a..f945f0450b16fd75f1ec1fc3c1dd81af9788b13c 100644 (file)
@@ -167,32 +167,4 @@ xfs_trans_ail_copy_lsn(
 }
 #endif
 
-static inline void
-xfs_clear_li_failed(
-       struct xfs_log_item     *lip)
-{
-       struct xfs_buf  *bp = lip->li_buf;
-
-       ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags));
-       lockdep_assert_held(&lip->li_ailp->ail_lock);
-
-       if (test_and_clear_bit(XFS_LI_FAILED, &lip->li_flags)) {
-               lip->li_buf = NULL;
-               xfs_buf_rele(bp);
-       }
-}
-
-static inline void
-xfs_set_li_failed(
-       struct xfs_log_item     *lip,
-       struct xfs_buf          *bp)
-{
-       lockdep_assert_held(&lip->li_ailp->ail_lock);
-
-       if (!test_and_set_bit(XFS_LI_FAILED, &lip->li_flags)) {
-               xfs_buf_hold(bp);
-               lip->li_buf = bp;
-       }
-}
-
 #endif /* __XFS_TRANS_PRIV_H__ */