xfs: refactor failed buffer resubmission into xfsaild

Flush locked log items whose underlying buffers fail metadata
writeback are tagged with a special flag to indicate that the flush
lock is already held. This is currently implemented in the type
specific ->iop_push() callback, but the processing required for such
items is not type specific because we're only doing basic state
management on the underlying buffer.

Factor the failed log item handling out of the inode and dquot
->iop_push() callbacks and open code the buffer resubmit helper into
a single helper called from xfsaild_push_item(). This provides a
generic mechanism for handling failed metadata buffer writeback with
a bit less code.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 1545657c3ca02a4e0924885c75b79ea9afa36377..8796adde2d12d39a1028725dab123574bbd2f4ce 100644 (file)
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -1248,42 +1248,3 @@ xfs_buf_iodone(
        xfs_trans_ail_delete(ailp, lip, SHUTDOWN_CORRUPT_INCORE);
        xfs_buf_item_free(BUF_ITEM(lip));
 }
-
-/*
- * Requeue a failed buffer for writeback.
- *
- * We clear the log item failed state here as well, but we have to be careful
- * about reference counts because the only active reference counts on the buffer
- * may be the failed log items. Hence if we clear the log item failed state
- * before queuing the buffer for IO we can release all active references to
- * the buffer and free it, leading to use after free problems in
- * xfs_buf_delwri_queue. It makes no difference to the buffer or log items which
- * order we process them in - the buffer is locked, and we own the buffer list
- * so nothing on them is going to change while we are performing this action.
- *
- * Hence we can safely queue the buffer for IO before we clear the failed log
- * item state, therefore  always having an active reference to the buffer and
- * avoiding the transient zero-reference state that leads to use-after-free.
- *
- * Return true if the buffer was added to the buffer list, false if it was
- * already on the buffer list.
- */
-bool
-xfs_buf_resubmit_failed_buffers(
-       struct xfs_buf          *bp,
-       struct list_head        *buffer_list)
-{
-       struct xfs_log_item     *lip;
-       bool                    ret;
-
-       ret = xfs_buf_delwri_queue(bp, buffer_list);
-
-       /*
-        * XFS_LI_FAILED set/clear is protected by ail_lock, caller of this
-        * function already have it acquired
-        */
-       list_for_each_entry(lip, &bp->b_li_list, li_bio_list)
-               xfs_clear_li_failed(lip);
-
-       return ret;
-}

diff --git a/fs/xfs/xfs_buf_item.h b/fs/xfs/xfs_buf_item.h
index 30114b5103325bd091682d3de7daaa24bf52758e..c9c57e2da9327cf5f6f3988c69e33612d1a841fc 100644 (file)
--- a/fs/xfs/xfs_buf_item.h
+++ b/fs/xfs/xfs_buf_item.h
@@ -59,8 +59,6 @@ void  xfs_buf_attach_iodone(struct xfs_buf *,
                              struct xfs_log_item *);
 void   xfs_buf_iodone_callbacks(struct xfs_buf *);
 void   xfs_buf_iodone(struct xfs_buf *, struct xfs_log_item *);
-bool   xfs_buf_resubmit_failed_buffers(struct xfs_buf *,
-                                       struct list_head *);
 bool   xfs_buf_log_check_iovec(struct xfs_log_iovec *iovec);
 
 extern kmem_zone_t     *xfs_buf_item_zone;

diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index baad1748d0d101eef191fb981619c918241cb539..5a7808299a320397a66117683a4276af3701740c 100644 (file)
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -145,21 +145,6 @@ xfs_qm_dquot_logitem_push(
        if (atomic_read(&dqp->q_pincount) > 0)
                return XFS_ITEM_PINNED;
 
-       /*
-        * The buffer containing this item failed to be written back
-        * previously. Resubmit the buffer for IO
-        */
-       if (test_bit(XFS_LI_FAILED, &lip->li_flags)) {
-               if (!xfs_buf_trylock(bp))
-                       return XFS_ITEM_LOCKED;
-
-               if (!xfs_buf_resubmit_failed_buffers(bp, buffer_list))
-                       rval = XFS_ITEM_FLUSHING;
-
-               xfs_buf_unlock(bp);
-               return rval;
-       }
-
        if (!xfs_dqlock_nowait(dqp))
                return XFS_ITEM_LOCKED;
 

diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 75b74bbe38e43227bb8ec275fe32f52e81985a14..a4027f4ca6c46c0f9aeb0cf36f093094171e875e 100644 (file)
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -497,21 +497,6 @@ xfs_inode_item_push(
        if (xfs_ipincount(ip) > 0)
                return XFS_ITEM_PINNED;
 
-       /*
-        * The buffer containing this item failed to be written back
-        * previously. Resubmit the buffer for IO.
-        */
-       if (test_bit(XFS_LI_FAILED, &lip->li_flags)) {
-               if (!xfs_buf_trylock(bp))
-                       return XFS_ITEM_LOCKED;
-
-               if (!xfs_buf_resubmit_failed_buffers(bp, buffer_list))
-                       rval = XFS_ITEM_FLUSHING;
-
-               xfs_buf_unlock(bp);
-               return rval;
-       }
-
        if (!xfs_ilock_nowait(ip, XFS_ILOCK_SHARED))
                return XFS_ITEM_LOCKED;
 

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 564253550b754b9f06f5e861ee3b483c766a7ab5..2574d01e4a834c6b6404acca87f1f273bc540e2d 100644 (file)
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -345,6 +345,45 @@ xfs_ail_delete(
        xfs_trans_ail_cursor_clear(ailp, lip);
 }
 
+/*
+ * Requeue a failed buffer for writeback.
+ *
+ * We clear the log item failed state here as well, but we have to be careful
+ * about reference counts because the only active reference counts on the buffer
+ * may be the failed log items. Hence if we clear the log item failed state
+ * before queuing the buffer for IO we can release all active references to
+ * the buffer and free it, leading to use after free problems in
+ * xfs_buf_delwri_queue. It makes no difference to the buffer or log items which
+ * order we process them in - the buffer is locked, and we own the buffer list
+ * so nothing on them is going to change while we are performing this action.
+ *
+ * Hence we can safely queue the buffer for IO before we clear the failed log
+ * item state, therefore  always having an active reference to the buffer and
+ * avoiding the transient zero-reference state that leads to use-after-free.
+ */
+static inline int
+xfsaild_resubmit_item(
+       struct xfs_log_item     *lip,
+       struct list_head        *buffer_list)
+{
+       struct xfs_buf          *bp = lip->li_buf;
+
+       if (!xfs_buf_trylock(bp))
+               return XFS_ITEM_LOCKED;
+
+       if (!xfs_buf_delwri_queue(bp, buffer_list)) {
+               xfs_buf_unlock(bp);
+               return XFS_ITEM_FLUSHING;
+       }
+
+       /* protected by ail_lock */
+       list_for_each_entry(lip, &bp->b_li_list, li_bio_list)
+               xfs_clear_li_failed(lip);
+
+       xfs_buf_unlock(bp);
+       return XFS_ITEM_SUCCESS;
+}
+
 static inline uint
 xfsaild_push_item(
        struct xfs_ail          *ailp,
@@ -365,6 +404,8 @@ xfsaild_push_item(
         */
        if (!lip->li_ops->iop_push)
                return XFS_ITEM_PINNED;
+       if (test_bit(XFS_LI_FAILED, &lip->li_flags))
+               return xfsaild_resubmit_item(lip, &ailp->ail_buf_list);
        return lip->li_ops->iop_push(lip, &ailp->ail_buf_list);
 }