[14/30] xfs: unwind log item error flagging
diff mbox series

Message ID 20200601214251.4167140-15-david@fromorbit.com
State Superseded
Headers show
Series
  • xfs: rework inode flushing to make inode reclaim fully asynchronous
Related show

Commit Message

Dave Chinner June 1, 2020, 9:42 p.m. UTC
From: Dave Chinner <dchinner@redhat.com>

When an buffer IO error occurs, we want to mark all
the log items attached to the buffer as failed. Open code
the error handling loop so that we can modify the flagging for the
different types of objects directly and independently of each other.

This also allows us to remove the ->iop_error method from the log
item operations.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_buf_item.c   | 48 ++++++++++++-----------------------------
 fs/xfs/xfs_dquot_item.c | 18 ----------------
 fs/xfs/xfs_inode_item.c | 18 ----------------
 fs/xfs/xfs_trans.h      |  1 -
 4 files changed, 14 insertions(+), 71 deletions(-)

Comments

Darrick J. Wong June 2, 2020, 8:45 p.m. UTC | #1
On Tue, Jun 02, 2020 at 07:42:35AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> When an buffer IO error occurs, we want to mark all
> the log items attached to the buffer as failed. Open code
> the error handling loop so that we can modify the flagging for the
> different types of objects directly and independently of each other.
> 
> This also allows us to remove the ->iop_error method from the log
> item operations.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_buf_item.c   | 48 ++++++++++++-----------------------------
>  fs/xfs/xfs_dquot_item.c | 18 ----------------
>  fs/xfs/xfs_inode_item.c | 18 ----------------
>  fs/xfs/xfs_trans.h      |  1 -
>  4 files changed, 14 insertions(+), 71 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index b6995719e877b..2364a9aa2d71a 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -12,6 +12,7 @@
>  #include "xfs_bit.h"
>  #include "xfs_mount.h"
>  #include "xfs_trans.h"
> +#include "xfs_trans_priv.h"
>  #include "xfs_buf_item.h"
>  #include "xfs_inode.h"
>  #include "xfs_inode_item.h"
> @@ -955,37 +956,6 @@ xfs_buf_item_relse(
>  	xfs_buf_item_free(bip);
>  }
>  
> -/*
> - * Invoke the error state callback for each log item affected by the failed I/O.
> - *
> - * If a metadata buffer write fails with a non-permanent error, the buffer is
> - * eventually resubmitted and so the completion callbacks are not run. The error
> - * state may need to be propagated to the log items attached to the buffer,
> - * however, so the next AIL push of the item knows hot to handle it correctly.
> - */
> -STATIC void
> -xfs_buf_do_callbacks_fail(
> -	struct xfs_buf		*bp)
> -{
> -	struct xfs_ail		*ailp = bp->b_mount->m_ail;
> -	struct xfs_log_item	*lip;
> -
> -	/*
> -	 * Buffer log item errors are handled directly by xfs_buf_item_push()
> -	 * and xfs_buf_iodone_callback_error, and they have no IO error
> -	 * callbacks. Check only for items in b_li_list.
> -	 */
> -	if (list_empty(&bp->b_li_list))
> -		return;
> -
> -	spin_lock(&ailp->ail_lock);
> -	list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
> -		if (lip->li_ops->iop_error)
> -			lip->li_ops->iop_error(lip, bp);
> -	}
> -	spin_unlock(&ailp->ail_lock);
> -}
> -
>  static bool
>  xfs_buf_ioerror_sync(
>  	struct xfs_buf		*bp)
> @@ -1154,13 +1124,18 @@ xfs_buf_inode_iodone(
>  	struct xfs_buf		*bp)
>  {
>  	if (bp->b_error) {
> +		struct xfs_log_item *lip;
>  		int ret = xfs_buf_iodone_error(bp);
>  		if (!ret)

Hmm we probably need a blank line between these declarations and the
start of the if statements, right?  Granted, I should've put this
complaint in the previous patch.

Otherwise this looks fine,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  			goto finish_iodone;
>  		if (ret == 1)
>  			return;
>  		ASSERT(ret == 2);
> -		xfs_buf_do_callbacks_fail(bp);
> +		spin_lock(&bp->b_mount->m_ail->ail_lock);
> +		list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
> +			xfs_set_li_failed(lip, bp);
> +		}
> +		spin_unlock(&bp->b_mount->m_ail->ail_lock);
>  		xfs_buf_relse(bp);
>  		return;
>  	}
> @@ -1180,13 +1155,18 @@ xfs_buf_dquot_iodone(
>  	struct xfs_buf		*bp)
>  {
>  	if (bp->b_error) {
> +		struct xfs_log_item *lip;
>  		int ret = xfs_buf_iodone_error(bp);
>  		if (!ret)
>  			goto finish_iodone;
>  		if (ret == 1)
>  			return;
>  		ASSERT(ret == 2);
> -		xfs_buf_do_callbacks_fail(bp);
> +		spin_lock(&bp->b_mount->m_ail->ail_lock);
> +		list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
> +			xfs_set_li_failed(lip, bp);
> +		}
> +		spin_unlock(&bp->b_mount->m_ail->ail_lock);
>  		xfs_buf_relse(bp);
>  		return;
>  	}
> @@ -1216,7 +1196,7 @@ xfs_buf_iodone(
>  		if (ret == 1)
>  			return;
>  		ASSERT(ret == 2);
> -		xfs_buf_do_callbacks_fail(bp);
> +		ASSERT(list_empty(&bp->b_li_list));
>  		xfs_buf_relse(bp);
>  		return;
>  	}
> diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> index 349c92d26570c..d7e4de7151d7f 100644
> --- a/fs/xfs/xfs_dquot_item.c
> +++ b/fs/xfs/xfs_dquot_item.c
> @@ -113,23 +113,6 @@ xfs_qm_dqunpin_wait(
>  	wait_event(dqp->q_pinwait, (atomic_read(&dqp->q_pincount) == 0));
>  }
>  
> -/*
> - * Callback used to mark a buffer with XFS_LI_FAILED when items in the buffer
> - * have been failed during writeback
> - *
> - * this informs the AIL that the dquot is already flush locked on the next push,
> - * and acquires a hold on the buffer to ensure that it isn't reclaimed before
> - * dirty data makes it to disk.
> - */
> -STATIC void
> -xfs_dquot_item_error(
> -	struct xfs_log_item	*lip,
> -	struct xfs_buf		*bp)
> -{
> -	ASSERT(!completion_done(&DQUOT_ITEM(lip)->qli_dquot->q_flush));
> -	xfs_set_li_failed(lip, bp);
> -}
> -
>  STATIC uint
>  xfs_qm_dquot_logitem_push(
>  	struct xfs_log_item	*lip,
> @@ -216,7 +199,6 @@ static const struct xfs_item_ops xfs_dquot_item_ops = {
>  	.iop_release	= xfs_qm_dquot_logitem_release,
>  	.iop_committing	= xfs_qm_dquot_logitem_committing,
>  	.iop_push	= xfs_qm_dquot_logitem_push,
> -	.iop_error	= xfs_dquot_item_error
>  };
>  
>  /*
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 7049f2ae8d186..86c783dec2bac 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -464,23 +464,6 @@ xfs_inode_item_unpin(
>  		wake_up_bit(&ip->i_flags, __XFS_IPINNED_BIT);
>  }
>  
> -/*
> - * Callback used to mark a buffer with XFS_LI_FAILED when items in the buffer
> - * have been failed during writeback
> - *
> - * This informs the AIL that the inode is already flush locked on the next push,
> - * and acquires a hold on the buffer to ensure that it isn't reclaimed before
> - * dirty data makes it to disk.
> - */
> -STATIC void
> -xfs_inode_item_error(
> -	struct xfs_log_item	*lip,
> -	struct xfs_buf		*bp)
> -{
> -	ASSERT(xfs_isiflocked(INODE_ITEM(lip)->ili_inode));
> -	xfs_set_li_failed(lip, bp);
> -}
> -
>  STATIC uint
>  xfs_inode_item_push(
>  	struct xfs_log_item	*lip,
> @@ -619,7 +602,6 @@ static const struct xfs_item_ops xfs_inode_item_ops = {
>  	.iop_committed	= xfs_inode_item_committed,
>  	.iop_push	= xfs_inode_item_push,
>  	.iop_committing	= xfs_inode_item_committing,
> -	.iop_error	= xfs_inode_item_error
>  };
>  
>  
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 99a9ab9cab25b..b752501818d25 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -74,7 +74,6 @@ struct xfs_item_ops {
>  	void (*iop_committing)(struct xfs_log_item *, xfs_lsn_t commit_lsn);
>  	void (*iop_release)(struct xfs_log_item *);
>  	xfs_lsn_t (*iop_committed)(struct xfs_log_item *, xfs_lsn_t);
> -	void (*iop_error)(struct xfs_log_item *, xfs_buf_t *);
>  	int (*iop_recover)(struct xfs_log_item *lip, struct xfs_trans *tp);
>  	bool (*iop_match)(struct xfs_log_item *item, uint64_t id);
>  };
> -- 
> 2.26.2.761.g0e0b3e54be
>
Brian Foster June 3, 2020, 3:02 p.m. UTC | #2
On Tue, Jun 02, 2020 at 07:42:35AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> When an buffer IO error occurs, we want to mark all
> the log items attached to the buffer as failed. Open code
> the error handling loop so that we can modify the flagging for the
> different types of objects directly and independently of each other.
> 
> This also allows us to remove the ->iop_error method from the log
> item operations.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_buf_item.c   | 48 ++++++++++++-----------------------------
>  fs/xfs/xfs_dquot_item.c | 18 ----------------
>  fs/xfs/xfs_inode_item.c | 18 ----------------
>  fs/xfs/xfs_trans.h      |  1 -
>  4 files changed, 14 insertions(+), 71 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index b6995719e877b..2364a9aa2d71a 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -12,6 +12,7 @@
>  #include "xfs_bit.h"
>  #include "xfs_mount.h"
>  #include "xfs_trans.h"
> +#include "xfs_trans_priv.h"
>  #include "xfs_buf_item.h"
>  #include "xfs_inode.h"
>  #include "xfs_inode_item.h"
> @@ -955,37 +956,6 @@ xfs_buf_item_relse(
>  	xfs_buf_item_free(bip);
>  }
>  
> -/*
> - * Invoke the error state callback for each log item affected by the failed I/O.
> - *
> - * If a metadata buffer write fails with a non-permanent error, the buffer is
> - * eventually resubmitted and so the completion callbacks are not run. The error
> - * state may need to be propagated to the log items attached to the buffer,
> - * however, so the next AIL push of the item knows hot to handle it correctly.
> - */
> -STATIC void
> -xfs_buf_do_callbacks_fail(
> -	struct xfs_buf		*bp)
> -{
> -	struct xfs_ail		*ailp = bp->b_mount->m_ail;
> -	struct xfs_log_item	*lip;
> -
> -	/*
> -	 * Buffer log item errors are handled directly by xfs_buf_item_push()
> -	 * and xfs_buf_iodone_callback_error, and they have no IO error
> -	 * callbacks. Check only for items in b_li_list.
> -	 */
> -	if (list_empty(&bp->b_li_list))
> -		return;
> -
> -	spin_lock(&ailp->ail_lock);
> -	list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
> -		if (lip->li_ops->iop_error)
> -			lip->li_ops->iop_error(lip, bp);
> -	}
> -	spin_unlock(&ailp->ail_lock);
> -}
> -
>  static bool
>  xfs_buf_ioerror_sync(
>  	struct xfs_buf		*bp)
> @@ -1154,13 +1124,18 @@ xfs_buf_inode_iodone(
>  	struct xfs_buf		*bp)
>  {
>  	if (bp->b_error) {
> +		struct xfs_log_item *lip;
>  		int ret = xfs_buf_iodone_error(bp);
>  		if (!ret)
>  			goto finish_iodone;
>  		if (ret == 1)
>  			return;
>  		ASSERT(ret == 2);
> -		xfs_buf_do_callbacks_fail(bp);
> +		spin_lock(&bp->b_mount->m_ail->ail_lock);
> +		list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
> +			xfs_set_li_failed(lip, bp);
> +		}
> +		spin_unlock(&bp->b_mount->m_ail->ail_lock);
>  		xfs_buf_relse(bp);
>  		return;
>  	}
> @@ -1180,13 +1155,18 @@ xfs_buf_dquot_iodone(
>  	struct xfs_buf		*bp)
>  {
>  	if (bp->b_error) {
> +		struct xfs_log_item *lip;
>  		int ret = xfs_buf_iodone_error(bp);
>  		if (!ret)
>  			goto finish_iodone;
>  		if (ret == 1)
>  			return;
>  		ASSERT(ret == 2);
> -		xfs_buf_do_callbacks_fail(bp);
> +		spin_lock(&bp->b_mount->m_ail->ail_lock);
> +		list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
> +			xfs_set_li_failed(lip, bp);
> +		}
> +		spin_unlock(&bp->b_mount->m_ail->ail_lock);
>  		xfs_buf_relse(bp);
>  		return;
>  	}
> @@ -1216,7 +1196,7 @@ xfs_buf_iodone(
>  		if (ret == 1)
>  			return;
>  		ASSERT(ret == 2);
> -		xfs_buf_do_callbacks_fail(bp);
> +		ASSERT(list_empty(&bp->b_li_list));
>  		xfs_buf_relse(bp);
>  		return;
>  	}
> diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> index 349c92d26570c..d7e4de7151d7f 100644
> --- a/fs/xfs/xfs_dquot_item.c
> +++ b/fs/xfs/xfs_dquot_item.c
> @@ -113,23 +113,6 @@ xfs_qm_dqunpin_wait(
>  	wait_event(dqp->q_pinwait, (atomic_read(&dqp->q_pincount) == 0));
>  }
>  
> -/*
> - * Callback used to mark a buffer with XFS_LI_FAILED when items in the buffer
> - * have been failed during writeback
> - *
> - * this informs the AIL that the dquot is already flush locked on the next push,
> - * and acquires a hold on the buffer to ensure that it isn't reclaimed before
> - * dirty data makes it to disk.
> - */
> -STATIC void
> -xfs_dquot_item_error(
> -	struct xfs_log_item	*lip,
> -	struct xfs_buf		*bp)
> -{
> -	ASSERT(!completion_done(&DQUOT_ITEM(lip)->qli_dquot->q_flush));
> -	xfs_set_li_failed(lip, bp);
> -}
> -
>  STATIC uint
>  xfs_qm_dquot_logitem_push(
>  	struct xfs_log_item	*lip,
> @@ -216,7 +199,6 @@ static const struct xfs_item_ops xfs_dquot_item_ops = {
>  	.iop_release	= xfs_qm_dquot_logitem_release,
>  	.iop_committing	= xfs_qm_dquot_logitem_committing,
>  	.iop_push	= xfs_qm_dquot_logitem_push,
> -	.iop_error	= xfs_dquot_item_error
>  };
>  
>  /*
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 7049f2ae8d186..86c783dec2bac 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -464,23 +464,6 @@ xfs_inode_item_unpin(
>  		wake_up_bit(&ip->i_flags, __XFS_IPINNED_BIT);
>  }
>  
> -/*
> - * Callback used to mark a buffer with XFS_LI_FAILED when items in the buffer
> - * have been failed during writeback
> - *
> - * This informs the AIL that the inode is already flush locked on the next push,
> - * and acquires a hold on the buffer to ensure that it isn't reclaimed before
> - * dirty data makes it to disk.
> - */
> -STATIC void
> -xfs_inode_item_error(
> -	struct xfs_log_item	*lip,
> -	struct xfs_buf		*bp)
> -{
> -	ASSERT(xfs_isiflocked(INODE_ITEM(lip)->ili_inode));
> -	xfs_set_li_failed(lip, bp);
> -}
> -
>  STATIC uint
>  xfs_inode_item_push(
>  	struct xfs_log_item	*lip,
> @@ -619,7 +602,6 @@ static const struct xfs_item_ops xfs_inode_item_ops = {
>  	.iop_committed	= xfs_inode_item_committed,
>  	.iop_push	= xfs_inode_item_push,
>  	.iop_committing	= xfs_inode_item_committing,
> -	.iop_error	= xfs_inode_item_error
>  };
>  
>  
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 99a9ab9cab25b..b752501818d25 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -74,7 +74,6 @@ struct xfs_item_ops {
>  	void (*iop_committing)(struct xfs_log_item *, xfs_lsn_t commit_lsn);
>  	void (*iop_release)(struct xfs_log_item *);
>  	xfs_lsn_t (*iop_committed)(struct xfs_log_item *, xfs_lsn_t);
> -	void (*iop_error)(struct xfs_log_item *, xfs_buf_t *);
>  	int (*iop_recover)(struct xfs_log_item *lip, struct xfs_trans *tp);
>  	bool (*iop_match)(struct xfs_log_item *item, uint64_t id);
>  };
> -- 
> 2.26.2.761.g0e0b3e54be
>

Patch
diff mbox series

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index b6995719e877b..2364a9aa2d71a 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -12,6 +12,7 @@ 
 #include "xfs_bit.h"
 #include "xfs_mount.h"
 #include "xfs_trans.h"
+#include "xfs_trans_priv.h"
 #include "xfs_buf_item.h"
 #include "xfs_inode.h"
 #include "xfs_inode_item.h"
@@ -955,37 +956,6 @@  xfs_buf_item_relse(
 	xfs_buf_item_free(bip);
 }
 
-/*
- * Invoke the error state callback for each log item affected by the failed I/O.
- *
- * If a metadata buffer write fails with a non-permanent error, the buffer is
- * eventually resubmitted and so the completion callbacks are not run. The error
- * state may need to be propagated to the log items attached to the buffer,
- * however, so the next AIL push of the item knows hot to handle it correctly.
- */
-STATIC void
-xfs_buf_do_callbacks_fail(
-	struct xfs_buf		*bp)
-{
-	struct xfs_ail		*ailp = bp->b_mount->m_ail;
-	struct xfs_log_item	*lip;
-
-	/*
-	 * Buffer log item errors are handled directly by xfs_buf_item_push()
-	 * and xfs_buf_iodone_callback_error, and they have no IO error
-	 * callbacks. Check only for items in b_li_list.
-	 */
-	if (list_empty(&bp->b_li_list))
-		return;
-
-	spin_lock(&ailp->ail_lock);
-	list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
-		if (lip->li_ops->iop_error)
-			lip->li_ops->iop_error(lip, bp);
-	}
-	spin_unlock(&ailp->ail_lock);
-}
-
 static bool
 xfs_buf_ioerror_sync(
 	struct xfs_buf		*bp)
@@ -1154,13 +1124,18 @@  xfs_buf_inode_iodone(
 	struct xfs_buf		*bp)
 {
 	if (bp->b_error) {
+		struct xfs_log_item *lip;
 		int ret = xfs_buf_iodone_error(bp);
 		if (!ret)
 			goto finish_iodone;
 		if (ret == 1)
 			return;
 		ASSERT(ret == 2);
-		xfs_buf_do_callbacks_fail(bp);
+		spin_lock(&bp->b_mount->m_ail->ail_lock);
+		list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
+			xfs_set_li_failed(lip, bp);
+		}
+		spin_unlock(&bp->b_mount->m_ail->ail_lock);
 		xfs_buf_relse(bp);
 		return;
 	}
@@ -1180,13 +1155,18 @@  xfs_buf_dquot_iodone(
 	struct xfs_buf		*bp)
 {
 	if (bp->b_error) {
+		struct xfs_log_item *lip;
 		int ret = xfs_buf_iodone_error(bp);
 		if (!ret)
 			goto finish_iodone;
 		if (ret == 1)
 			return;
 		ASSERT(ret == 2);
-		xfs_buf_do_callbacks_fail(bp);
+		spin_lock(&bp->b_mount->m_ail->ail_lock);
+		list_for_each_entry(lip, &bp->b_li_list, li_bio_list) {
+			xfs_set_li_failed(lip, bp);
+		}
+		spin_unlock(&bp->b_mount->m_ail->ail_lock);
 		xfs_buf_relse(bp);
 		return;
 	}
@@ -1216,7 +1196,7 @@  xfs_buf_iodone(
 		if (ret == 1)
 			return;
 		ASSERT(ret == 2);
-		xfs_buf_do_callbacks_fail(bp);
+		ASSERT(list_empty(&bp->b_li_list));
 		xfs_buf_relse(bp);
 		return;
 	}
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index 349c92d26570c..d7e4de7151d7f 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -113,23 +113,6 @@  xfs_qm_dqunpin_wait(
 	wait_event(dqp->q_pinwait, (atomic_read(&dqp->q_pincount) == 0));
 }
 
-/*
- * Callback used to mark a buffer with XFS_LI_FAILED when items in the buffer
- * have been failed during writeback
- *
- * this informs the AIL that the dquot is already flush locked on the next push,
- * and acquires a hold on the buffer to ensure that it isn't reclaimed before
- * dirty data makes it to disk.
- */
-STATIC void
-xfs_dquot_item_error(
-	struct xfs_log_item	*lip,
-	struct xfs_buf		*bp)
-{
-	ASSERT(!completion_done(&DQUOT_ITEM(lip)->qli_dquot->q_flush));
-	xfs_set_li_failed(lip, bp);
-}
-
 STATIC uint
 xfs_qm_dquot_logitem_push(
 	struct xfs_log_item	*lip,
@@ -216,7 +199,6 @@  static const struct xfs_item_ops xfs_dquot_item_ops = {
 	.iop_release	= xfs_qm_dquot_logitem_release,
 	.iop_committing	= xfs_qm_dquot_logitem_committing,
 	.iop_push	= xfs_qm_dquot_logitem_push,
-	.iop_error	= xfs_dquot_item_error
 };
 
 /*
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 7049f2ae8d186..86c783dec2bac 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -464,23 +464,6 @@  xfs_inode_item_unpin(
 		wake_up_bit(&ip->i_flags, __XFS_IPINNED_BIT);
 }
 
-/*
- * Callback used to mark a buffer with XFS_LI_FAILED when items in the buffer
- * have been failed during writeback
- *
- * This informs the AIL that the inode is already flush locked on the next push,
- * and acquires a hold on the buffer to ensure that it isn't reclaimed before
- * dirty data makes it to disk.
- */
-STATIC void
-xfs_inode_item_error(
-	struct xfs_log_item	*lip,
-	struct xfs_buf		*bp)
-{
-	ASSERT(xfs_isiflocked(INODE_ITEM(lip)->ili_inode));
-	xfs_set_li_failed(lip, bp);
-}
-
 STATIC uint
 xfs_inode_item_push(
 	struct xfs_log_item	*lip,
@@ -619,7 +602,6 @@  static const struct xfs_item_ops xfs_inode_item_ops = {
 	.iop_committed	= xfs_inode_item_committed,
 	.iop_push	= xfs_inode_item_push,
 	.iop_committing	= xfs_inode_item_committing,
-	.iop_error	= xfs_inode_item_error
 };
 
 
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 99a9ab9cab25b..b752501818d25 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -74,7 +74,6 @@  struct xfs_item_ops {
 	void (*iop_committing)(struct xfs_log_item *, xfs_lsn_t commit_lsn);
 	void (*iop_release)(struct xfs_log_item *);
 	xfs_lsn_t (*iop_committed)(struct xfs_log_item *, xfs_lsn_t);
-	void (*iop_error)(struct xfs_log_item *, xfs_buf_t *);
 	int (*iop_recover)(struct xfs_log_item *lip, struct xfs_trans *tp);
 	bool (*iop_match)(struct xfs_log_item *item, uint64_t id);
 };