Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1733991

[PATCH 4.13 29/52] xfs: Properly retry failed inode items in case of error during buffer writeback

From Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Newsgroups linux.kernel
Subject [PATCH 4.13 29/52] xfs: Properly retry failed inode items in case of error during buffer writeback
Date 2017-09-18 12:00 +0200
Message-ID <ur10p-4sC-87@gated-at.bofh.it> (permalink)
References <ur0nE-4ck-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


4.13-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Carlos Maiolino <cmaiolino@redhat.com>

commit d3a304b6292168b83b45d624784f973fdc1ca674 upstream.

When a buffer has been failed during writeback, the inode items into it
are kept flush locked, and are never resubmitted due the flush lock, so,
if any buffer fails to be written, the items in AIL are never written to
disk and never unlocked.

This causes unmount operation to hang due these items flush locked in AIL,
but this also causes the items in AIL to never be written back, even when
the IO device comes back to normal.

I've been testing this patch with a DM-thin device, creating a
filesystem larger than the real device.

When writing enough data to fill the DM-thin device, XFS receives ENOSPC
errors from the device, and keep spinning on xfsaild (when 'retry
forever' configuration is set).

At this point, the filesystem can not be unmounted because of the flush locked
items in AIL, but worse, the items in AIL are never retried at all
(once xfs_inode_item_push() will skip the items that are flush locked),
even if the underlying DM-thin device is expanded to the proper size.

This patch fixes both cases, retrying any item that has been failed
previously, using the infra-structure provided by the previous patch.

Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/xfs/xfs_buf_item.c   |   28 ++++++++++++++++++++++++++++
 fs/xfs/xfs_buf_item.h   |    3 +++
 fs/xfs/xfs_inode_item.c |   47 +++++++++++++++++++++++++++++++++++++++++++----
 fs/xfs/xfs_trans.h      |    1 +
 fs/xfs/xfs_trans_ail.c  |    3 ++-
 fs/xfs/xfs_trans_priv.h |   31 +++++++++++++++++++++++++++++++
 6 files changed, 108 insertions(+), 5 deletions(-)

--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -1234,3 +1234,31 @@ 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
+ *
+ * Return true if the buffer has been re-queued properly, false otherwise
+ */
+bool
+xfs_buf_resubmit_failed_buffers(
+	struct xfs_buf		*bp,
+	struct xfs_log_item	*lip,
+	struct list_head	*buffer_list)
+{
+	struct xfs_log_item	*next;
+
+	/*
+	 * Clear XFS_LI_FAILED flag from all items before resubmit
+	 *
+	 * XFS_LI_FAILED set/clear is protected by xa_lock, caller  this
+	 * function already have it acquired
+	 */
+	for (; lip; lip = next) {
+		next = lip->li_bio_list;
+		xfs_clear_li_failed(lip);
+	}
+
+	/* Add this buffer back to the delayed write list */
+	return xfs_buf_delwri_queue(bp, buffer_list);
+}
--- a/fs/xfs/xfs_buf_item.h
+++ b/fs/xfs/xfs_buf_item.h
@@ -70,6 +70,9 @@ void	xfs_buf_attach_iodone(struct xfs_bu
 			      xfs_log_item_t *);
 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 xfs_log_item *,
+					struct list_head *);
 
 extern kmem_zone_t	*xfs_buf_item_zone;
 
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -27,6 +27,7 @@
 #include "xfs_error.h"
 #include "xfs_trace.h"
 #include "xfs_trans_priv.h"
+#include "xfs_buf_item.h"
 #include "xfs_log.h"
 
 
@@ -475,6 +476,23 @@ 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,
@@ -484,13 +502,28 @@ xfs_inode_item_push(
 {
 	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
 	struct xfs_inode	*ip = iip->ili_inode;
-	struct xfs_buf		*bp = NULL;
+	struct xfs_buf		*bp = lip->li_buf;
 	uint			rval = XFS_ITEM_SUCCESS;
 	int			error;
 
 	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 (lip->li_flags & XFS_LI_FAILED) {
+		if (!xfs_buf_trylock(bp))
+			return XFS_ITEM_LOCKED;
+
+		if (!xfs_buf_resubmit_failed_buffers(bp, lip, buffer_list))
+			rval = XFS_ITEM_FLUSHING;
+
+		xfs_buf_unlock(bp);
+		return rval;
+	}
+
 	if (!xfs_ilock_nowait(ip, XFS_ILOCK_SHARED))
 		return XFS_ITEM_LOCKED;
 
@@ -622,7 +655,8 @@ static const struct xfs_item_ops xfs_ino
 	.iop_unlock	= xfs_inode_item_unlock,
 	.iop_committed	= xfs_inode_item_committed,
 	.iop_push	= xfs_inode_item_push,
-	.iop_committing = xfs_inode_item_committing
+	.iop_committing = xfs_inode_item_committing,
+	.iop_error	= xfs_inode_item_error
 };
 
 
@@ -710,7 +744,8 @@ xfs_iflush_done(
 		 * the AIL lock.
 		 */
 		iip = INODE_ITEM(blip);
-		if (iip->ili_logged && blip->li_lsn == iip->ili_flush_lsn)
+		if ((iip->ili_logged && blip->li_lsn == iip->ili_flush_lsn) ||
+		    lip->li_flags & XFS_LI_FAILED)
 			need_ail++;
 
 		blip = next;
@@ -718,7 +753,8 @@ xfs_iflush_done(
 
 	/* make sure we capture the state of the initial inode. */
 	iip = INODE_ITEM(lip);
-	if (iip->ili_logged && lip->li_lsn == iip->ili_flush_lsn)
+	if ((iip->ili_logged && lip->li_lsn == iip->ili_flush_lsn) ||
+	    lip->li_flags & XFS_LI_FAILED)
 		need_ail++;
 
 	/*
@@ -739,6 +775,9 @@ xfs_iflush_done(
 			if (INODE_ITEM(blip)->ili_logged &&
 			    blip->li_lsn == INODE_ITEM(blip)->ili_flush_lsn)
 				mlip_changed |= xfs_ail_delete_one(ailp, blip);
+			else {
+				xfs_clear_li_failed(blip);
+			}
 		}
 
 		if (mlip_changed) {
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -49,6 +49,7 @@ typedef struct xfs_log_item {
 	struct xfs_ail			*li_ailp;	/* ptr to AIL */
 	uint				li_type;	/* item type */
 	uint				li_flags;	/* misc flags */
+	struct xfs_buf			*li_buf;	/* real buffer pointer */
 	struct xfs_log_item		*li_bio_list;	/* buffer item list */
 	void				(*li_cb)(struct xfs_buf *,
 						 struct xfs_log_item *);
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -687,12 +687,13 @@ xfs_trans_ail_update_bulk(
 bool
 xfs_ail_delete_one(
 	struct xfs_ail		*ailp,
-	struct xfs_log_item 	*lip)
+	struct xfs_log_item	*lip)
 {
 	struct xfs_log_item	*mlip = xfs_ail_min(ailp);
 
 	trace_xfs_ail_delete(lip, mlip->li_lsn, lip->li_lsn);
 	xfs_ail_delete(ailp, lip);
+	xfs_clear_li_failed(lip);
 	lip->li_flags &= ~XFS_LI_IN_AIL;
 	lip->li_lsn = 0;
 
--- a/fs/xfs/xfs_trans_priv.h
+++ b/fs/xfs/xfs_trans_priv.h
@@ -164,4 +164,35 @@ xfs_trans_ail_copy_lsn(
 	*dst = *src;
 }
 #endif
+
+static inline void
+xfs_clear_li_failed(
+	struct xfs_log_item	*lip)
+{
+	struct xfs_buf	*bp = lip->li_buf;
+
+	ASSERT(lip->li_flags & XFS_LI_IN_AIL);
+	lockdep_assert_held(&lip->li_ailp->xa_lock);
+
+	if (lip->li_flags & XFS_LI_FAILED) {
+		lip->li_flags &= ~XFS_LI_FAILED;
+		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->xa_lock);
+
+	if (!(lip->li_flags & XFS_LI_FAILED)) {
+		xfs_buf_hold(bp);
+		lip->li_flags |= XFS_LI_FAILED;
+		lip->li_buf = bp;
+	}
+}
+
 #endif	/* __XFS_TRANS_PRIV_H__ */

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 4.13 00/52] 4.13.3-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 14/52] thunderbolt: Remove superfluous check Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 16/52] thunderbolt: Allow clearing the key Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 30/52] xfs: fix recovery failure when log record header wraps log end Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 04/52] vhost_net: correctly check tx avail during rx busy polling Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 20/52] x86/mm, mm/hwpoison: Clear PRESENT bit for kernel 1:1 mappings of poison pages Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 35/52] xfs: evict all inodes involved with log redo item Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 21/52] ovl: fix false positive ESTALE on lookup Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 37/52] xfs: open-code xfs_buf_item_dirty() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 11/52] ip_tunnel: fix setting ttl and tos value in collect_md mode Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 01/52] Revert "net: use lib/percpu_counter API for fragmentation mem accounting" Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 12/52] f2fs: let fill_super handle roll-forward errors Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 15/52] thunderbolt: Make key root-only accessible Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 47/52] xfs: fix incorrect log_flushed on fsync Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 40/52] xfs: refactor buffer logging into buffer dirtying helper Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 52/52] md/raid5: release/flush io in raid5_do_work() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 11:20 +0200
  [PATCH 4.13 08/52] ipv6: fix typo in fib6_net_exit() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 50/52] xfs: use kmem_free to free return value of kmem_zalloc Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 28/52] xfs: Add infrastructure needed for error propagation during buffer IO failure Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 24/52] libnvdimm, btt: check memory allocation failure Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 38/52] xfs: remove unnecessary dirty bli format check for ordered bufs Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 46/52] xfs: disable per-inode DAX flag Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 23/52] idr: remove WARN_ON_ONCE() when trying to replace negative ID Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 31/52] xfs: always verify the log tail during recovery Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 41/52] xfs: dont log dirty ranges for ordered buffers Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 43/52] xfs: move bmbt owner change to last step of extent swap Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 48/52] xfs: dont set v3 xflags for v2 inodes Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 13/52] f2fs: check hot_data for roll-forward recovery Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 33/52] xfs: handle -EFSCORRUPTED during head/tail verification Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 39/52] xfs: ordered buffer log items are never formatted Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 10/52] tcp: fix a request socket leak Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 34/52] xfs: stop searching for free slots in an inode chunk when there are none Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 09/52] sctp: fix missing wake ups in some situations Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 42/52] xfs: skip bmbt block ino validation during owner change Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 51/52] md/raid1/10: reset bio allocated from mempool Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 36/52] xfs: check for race with xfs_reclaim_inode() in xfs_ifree_cluster() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 25/52] libnvdimm: fix integer overflow static analysis warning Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 32/52] xfs: fix log recovery corruption error due to tail overwrite Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 29/52] xfs: Properly retry failed inode items in case of error during buffer writeback Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:00 +0200
  [PATCH 4.13 03/52] gianfar: Fix Tx flow control deactivation Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:10 +0200
  [PATCH 4.13 02/52] Revert "net: fix percpu memory leaks" Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:10 +0200
  [PATCH 4.13 17/52] x86/fsgsbase/64: Fully initialize FS and GS state in start_thread_common Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:10 +0200
  [PATCH 4.13 06/52] udp: drop head states only when all skb references are gone Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-18 12:10 +0200
  Re: [PATCH 4.13 00/52] 4.13.3-stable review Guenter Roeck <linux@roeck-us.net> - 2017-09-18 21:30 +0200
  Re: [PATCH 4.13 00/52] 4.13.3-stable review Shuah Khan <shuahkh@osg.samsung.com> - 2017-09-18 22:20 +0200
    Re: [PATCH 4.13 00/52] 4.13.3-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-09-19 08:40 +0200

csiph-web