public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Mark Tinguely <tinguely@sgi.com>
To: xfs@oss.sgi.com
Subject: [PATCH 2/5] xfs: free the EFI entries from AIL on forced shutdown
Date: Wed, 02 Jul 2014 09:32:08 -0500	[thread overview]
Message-ID: <20140702144139.705337423@sgi.com> (raw)
In-Reply-To: 20140702143206.438456679@sgi.com

[-- Attachment #1: xfs-fix-efi-on-filesystem-errors.patch --]
[-- Type: text/plain, Size: 4304 bytes --]

The extent free intention (EFI) entry and extent free done (EFD)
log items are in separate transactions. It is possible that
the EFI can be pushed to the AIL before a forced shutdown
where it gets stuck on the AIL and requires a reboot in the
following places:

1) xfs_bmap_finish:
  The EFD is not in the transaction. This can happen if the
  transaction in xfs_bmap_finish() fails to reserve space.

  The EFD will not be dirty in the transaction if the error
  happened while freeing the first extent in the list.

2) EFD IOP Abort processing:
  If xfs_trans_cancel() is called with an abort flag, or if the
  xfs_trans_commit() is called when the file system is in forced
  shutdown or if the log buffer write fails.

 
Do not free the EFI structure immediately on forced shutdowns, but
instead use the IOP calls to match the EFI/EFD entries. A small
wrinkle in the mix is the EFI is not automatically placed on the
AIL by the IOP routines in the cases but may have made it to the
AIL before the abort. We now have to check if the EFI is on the AIL
in the abort IOP cases.

Remove the debug code, because the EFD could be in the log item list
on a xfs_trans_cancel.

Signed-off-by: Mark Tinguely <tinguely@sgi.com>
---
 fs/xfs/xfs_bmap_util.c |   36 ++++++++++++++++++------------------
 fs/xfs/xfs_trans.c     |    8 --------
 2 files changed, 18 insertions(+), 26 deletions(-)

Index: b/fs/xfs/xfs_bmap_util.c
===================================================================
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -79,7 +79,6 @@ xfs_bmap_finish(
 	int			error;		/* error return value */
 	xfs_bmap_free_item_t	*free;		/* free extent item */
 	struct xfs_trans_res	tres;		/* new log reservation */
-	xfs_mount_t		*mp;		/* filesystem mount structure */
 	xfs_bmap_free_item_t	*next;		/* next item on free list */
 	xfs_trans_t		*ntp;		/* new transaction pointer */
 
@@ -115,31 +114,32 @@ xfs_bmap_finish(
 	xfs_log_ticket_put(ntp->t_ticket);
 
 	error = xfs_trans_reserve(ntp, &tres, 0, 0);
-	if (error)
+	if (error) {
+		/*
+		 * EFD is not in the transaction. Manually release the EFI
+		 * and remove it from AIL if necessary.
+		 */
+		xfs_efi_release(efi, efi->efi_format.efi_nextents);
 		return error;
+	}
+
 	efd = xfs_trans_get_efd(ntp, efi, flist->xbf_count);
 	for (free = flist->xbf_first; free != NULL; free = next) {
 		next = free->xbfi_next;
-		if ((error = xfs_free_extent(ntp, free->xbfi_startblock,
-				free->xbfi_blockcount))) {
+		error = xfs_free_extent(ntp, free->xbfi_startblock,
+				free->xbfi_blockcount);
+		xfs_trans_log_efd_extent(ntp, efd, free->xbfi_startblock,
+					 free->xbfi_blockcount);
+		if (error) {
 			/*
-			 * The bmap free list will be cleaned up at a
-			 * higher level.  The EFI will be canceled when
-			 * this transaction is aborted.
-			 * Need to force shutdown here to make sure it
-			 * happens, since this transaction may not be
-			 * dirty yet.
+			 * Encountered an error while freeing the extent.
+			 * The bmap free list will be cleaned up at a higher
+			 * level. The caller will call xfs_trans_cancel on
+			 * error, the filesystem will be shutdown and the
+			 * EFI and EFD will be freed.
 			 */
-			mp = ntp->t_mountp;
-			if (!XFS_FORCED_SHUTDOWN(mp))
-				xfs_force_shutdown(mp,
-						   (error == EFSCORRUPTED) ?
-						   SHUTDOWN_CORRUPT_INCORE :
-						   SHUTDOWN_META_IO_ERROR);
 			return error;
 		}
-		xfs_trans_log_efd_extent(ntp, efd, free->xbfi_startblock,
-			free->xbfi_blockcount);
 		xfs_bmap_del_free(flist, NULL, free);
 	}
 	return 0;
Index: b/fs/xfs/xfs_trans.c
===================================================================
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -958,14 +958,6 @@ xfs_trans_cancel(
 		XFS_ERROR_REPORT("xfs_trans_cancel", XFS_ERRLEVEL_LOW, mp);
 		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
 	}
-#ifdef DEBUG
-	if (!(flags & XFS_TRANS_ABORT) && !XFS_FORCED_SHUTDOWN(mp)) {
-		struct xfs_log_item_desc *lidp;
-
-		list_for_each_entry(lidp, &tp->t_items, lid_trans)
-			ASSERT(!(lidp->lid_item->li_type == XFS_LI_EFD));
-	}
-#endif
 	xfs_trans_unreserve_and_mod_sb(tp);
 	xfs_trans_unreserve_and_mod_dquots(tp);
 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  parent reply	other threads:[~2014-07-02 14:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-02 14:32 [PATCH 0/5] Misc controversial patches Mark Tinguely
2014-07-02 14:32 ` [PATCH 1/5] xfs: remove efi from AIL in log recovery Mark Tinguely
2014-07-07 14:30   ` Brian Foster
2014-07-07 15:29     ` Mark Tinguely
2014-07-07 23:44       ` Dave Chinner
2014-07-02 14:32 ` Mark Tinguely [this message]
2014-07-02 14:32 ` [PATCH 3/5] xfs: free the list of recovery items on error Mark Tinguely
2014-07-02 14:32 ` [PATCH 4/5] xfs: free inodes on log recovery error Mark Tinguely
2014-07-07 14:30   ` Brian Foster
2014-07-07 15:18     ` Mark Tinguely
2014-07-09  9:02   ` Christoph Hellwig
2014-07-02 14:32 ` [PATCH 5/5] xfs: fix cil push sequence after log recovery Mark Tinguely
2014-07-07 15:26   ` Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140702144139.705337423@sgi.com \
    --to=tinguely@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox