public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: xfs@oss.sgi.com
Subject: [PATCH 06/11] xfs: don't leave EFIs on AIL on mount failure
Date: Thu,  6 Aug 2015 13:44:27 -0400	[thread overview]
Message-ID: <1438883072-28706-7-git-send-email-bfoster@redhat.com> (raw)
In-Reply-To: <1438883072-28706-1-git-send-email-bfoster@redhat.com>

Log recovery occurs in two phases at mount time. In the first phase,
EFIs and EFDs are processed and potentially cancelled out. EFIs without
EFD objects are inserted into the AIL for processing and recovery in the
second phase. xfs_mountfs() runs various other operations between the
phases and is thus subject to failure. If failure occurs after the first
phase but before the second, pending EFIs sit on the AIL, pin it and
cause the mount to hang.

Update the mount sequence to ensure that pending EFIs are cancelled in
the event of failure. Add a recovery cancellation mechanism to iterate
the AIL and cancel all EFI items when requested. Plumb cancellation
support through the log mount finish helper and update xfs_mountfs() to
invoke cancellation in the event of failure after recovery has started.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_log.c         | 16 +++++++++++-----
 fs/xfs/xfs_log.h         |  2 +-
 fs/xfs/xfs_log_priv.h    |  2 ++
 fs/xfs/xfs_log_recover.c | 41 ++++++++++++++++++++++++++++++++++++-----
 fs/xfs/xfs_mount.c       | 35 +++++++++++++++++++++++------------
 5 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 6b5a84a..522286c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -740,19 +740,25 @@ out:
  * it.
  */
 int
-xfs_log_mount_finish(xfs_mount_t *mp)
+xfs_log_mount_finish(
+	struct xfs_mount	*mp,
+	bool			cancel)
 {
 	int	error = 0;
 
-	if (!(mp->m_flags & XFS_MOUNT_NORECOVERY)) {
+	if (mp->m_flags & XFS_MOUNT_NORECOVERY) {
+		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
+		return 0;
+	}
+
+	if (cancel) {
+		error = xlog_recover_cancel(mp->m_log);
+	} else {
 		error = xlog_recover_finish(mp->m_log);
 		if (!error)
 			xfs_log_work_queue(mp);
-	} else {
-		ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
 	}
 
-
 	return error;
 }
 
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index fa27aae..425e880 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -146,7 +146,7 @@ int	  xfs_log_mount(struct xfs_mount	*mp,
 			struct xfs_buftarg	*log_target,
 			xfs_daddr_t		start_block,
 			int		 	num_bblocks);
-int	  xfs_log_mount_finish(struct xfs_mount *mp);
+int	  xfs_log_mount_finish(struct xfs_mount *mp, bool);
 xfs_lsn_t xlog_assign_tail_lsn(struct xfs_mount *mp);
 xfs_lsn_t xlog_assign_tail_lsn_locked(struct xfs_mount *mp);
 void	  xfs_log_space_wake(struct xfs_mount *mp);
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 1c87c8a..950f3f9 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -426,6 +426,8 @@ xlog_recover(
 extern int
 xlog_recover_finish(
 	struct xlog		*log);
+extern int
+xlog_recover_cancel(struct xlog *);
 
 extern __le32	 xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead,
 			    char *dp, int size);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index a74ff68..a7ba078 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3823,10 +3823,11 @@ abort_error:
  */
 STATIC int
 xlog_recover_process_efis(
-	struct xlog	*log)
+	struct xlog		*log,
+	bool			cancel)
 {
-	xfs_log_item_t		*lip;
-	xfs_efi_log_item_t	*efip;
+	struct xfs_log_item	*lip;
+	struct xfs_efi_log_item	*efip;
 	int			error = 0;
 	struct xfs_ail_cursor	cur;
 	struct xfs_ail		*ailp;
@@ -3847,10 +3848,24 @@ xlog_recover_process_efis(
 			break;
 		}
 
+		efip = (struct xfs_efi_log_item *) lip;
+
+		/*
+		 * A cancel occurs when the mount has failed and we're bailing
+		 * out. Release all pending EFIs so they don't pin the AIL.
+		 */
+		if (cancel) {
+			spin_unlock(&ailp->xa_lock);
+			xfs_efi_release(efip);
+			spin_lock(&ailp->xa_lock);
+
+			lip = xfs_trans_ail_cursor_next(ailp, &cur);
+			continue;
+		}
+
 		/*
 		 * Skip EFIs that we've already processed.
 		 */
-		efip = (xfs_efi_log_item_t *)lip;
 		if (test_bit(XFS_EFI_RECOVERED, &efip->efi_flags)) {
 			lip = xfs_trans_ail_cursor_next(ailp, &cur);
 			continue;
@@ -4617,8 +4632,13 @@ xlog_recover_finish(
 	 */
 	if (log->l_flags & XLOG_RECOVERY_NEEDED) {
 		int	error;
-		error = xlog_recover_process_efis(log);
+		error = xlog_recover_process_efis(log, false);
 		if (error) {
+			/*
+			 * We could still have pending EFIs. Cancel them so we
+			 * don't hang...
+			 */
+			xlog_recover_process_efis(log, true);
 			xfs_alert(log->l_mp, "Failed to recover EFIs");
 			return error;
 		}
@@ -4644,6 +4664,17 @@ xlog_recover_finish(
 	return 0;
 }
 
+int
+xlog_recover_cancel(
+	struct xlog	*log)
+{
+	int		error = 0;
+
+	if (log->l_flags & XLOG_RECOVERY_NEEDED)
+		error = xlog_recover_process_efis(log, true);
+
+	return error;
+}
 
 #if defined(DEBUG)
 /*
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 461e791..45a33d5 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -615,14 +615,15 @@ xfs_default_resblks(xfs_mount_t *mp)
  */
 int
 xfs_mountfs(
-	xfs_mount_t	*mp)
+	struct xfs_mount	*mp)
 {
-	xfs_sb_t	*sbp = &(mp->m_sb);
-	xfs_inode_t	*rip;
-	__uint64_t	resblks;
-	uint		quotamount = 0;
-	uint		quotaflags = 0;
-	int		error = 0;
+	struct xfs_sb		*sbp = &(mp->m_sb);
+	struct xfs_inode	*rip;
+	__uint64_t		resblks;
+	uint			quotamount = 0;
+	uint			quotaflags = 0;
+	int			error = 0;
+	bool			log_mount_cancel = false;
 
 	xfs_sb_mount_common(mp, sbp);
 
@@ -799,7 +800,10 @@ xfs_mountfs(
 	}
 
 	/*
-	 * log's mount-time initialization. Perform 1st part recovery if needed
+	 * Log's mount-time initialization. The first part of recovery can place
+	 * some items on the AIL, to be handled when recovery is finished. Set
+	 * the cancel flag to ensure that the recovery is cancelled should we
+	 * fail before then.
 	 */
 	error = xfs_log_mount(mp, mp->m_logdev_targp,
 			      XFS_FSB_TO_DADDR(mp, sbp->sb_logstart),
@@ -808,6 +812,7 @@ xfs_mountfs(
 		xfs_warn(mp, "log mount failed");
 		goto out_fail_wait;
 	}
+	log_mount_cancel = true;
 
 	/*
 	 * Now the log is mounted, we know if it was an unclean shutdown or
@@ -910,11 +915,15 @@ xfs_mountfs(
 	}
 
 	/*
-	 * Finish recovering the file system.  This part needed to be
-	 * delayed until after the root and real-time bitmap inodes
-	 * were consistently read in.
+	 * Finish recovering the file system.  This part needed to be delayed
+	 * until after the root and real-time bitmap inodes were consistently
+	 * read in.
+	 *
+	 * Reset the cancel flag as the finish cleans up after itself on
+	 * failure.
 	 */
-	error = xfs_log_mount_finish(mp);
+	log_mount_cancel = false;
+	error = xfs_log_mount_finish(mp, false);
 	if (error) {
 		xfs_warn(mp, "log mount finish failed");
 		goto out_rtunmount;
@@ -956,6 +965,8 @@ xfs_mountfs(
  out_rele_rip:
 	IRELE(rip);
  out_log_dealloc:
+	if (log_mount_cancel)
+		xfs_log_mount_finish(mp, true);
 	xfs_log_unmount(mp);
  out_fail_wait:
 	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
-- 
2.1.0

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

  parent reply	other threads:[~2015-08-06 17:44 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-06 17:44 [PATCH 00/11] xfs: miscellaneous logging, recovery, umount fixes Brian Foster
2015-08-06 17:44 ` [PATCH 01/11] xfs: disentagle EFI release from the extent count Brian Foster
2015-08-09  7:36   ` Christoph Hellwig
2015-08-10 12:37     ` Brian Foster
2015-08-12  7:15       ` Christoph Hellwig
2015-08-12 12:47         ` Brian Foster
2015-08-06 17:44 ` [PATCH 02/11] xfs: return committed status from xfs_trans_roll() Brian Foster
2015-08-09  7:37   ` Christoph Hellwig
2015-08-06 17:44 ` [PATCH 03/11] xfs: fix efi/efd error handling to avoid fs shutdown hangs Brian Foster
2015-08-07  0:41   ` Dave Chinner
2015-08-07 12:09     ` Brian Foster
2015-08-07 22:45       ` Dave Chinner
2015-08-09  7:46   ` Christoph Hellwig
2015-08-06 17:44 ` [PATCH 04/11] xfs: ensure EFD trans aborts on log recovery extent free failure Brian Foster
2015-08-09  7:53   ` Christoph Hellwig
2015-08-10 12:38     ` Brian Foster
2015-08-10 13:39       ` Brian Foster
2015-08-06 17:44 ` [PATCH 05/11] xfs: use EFI refcount consistently in log recovery Brian Foster
2015-08-07  1:51   ` Dave Chinner
2015-08-07 12:09     ` Brian Foster
2015-08-09  7:56   ` Christoph Hellwig
2015-08-10 12:37     ` Brian Foster
2015-08-06 17:44 ` Brian Foster [this message]
2015-08-07  2:23   ` [PATCH 06/11] xfs: don't leave EFIs on AIL on mount failure Dave Chinner
2015-08-07 12:10     ` Brian Foster
2015-08-09  8:01   ` Christoph Hellwig
2015-08-10 12:38     ` Brian Foster
2015-08-06 17:44 ` [PATCH 07/11] xfs: icreate log item recovery and cancellation tracepoints Brian Foster
2015-08-09  8:02   ` Christoph Hellwig
2015-08-06 17:44 ` [PATCH 08/11] xfs: fix broken icreate log item cancellation Brian Foster
2015-08-06 17:44 ` [PATCH 09/11] xfs: checksum log record ext headers based on record size Brian Foster
2015-08-06 17:44 ` [PATCH 10/11] xfs: clean up root inode properly on mount failure Brian Foster
2015-08-09  8:03   ` Christoph Hellwig
2015-08-06 17:44 ` [PATCH 11/11] xfs: fix btree cursor error cleanups Brian Foster
2015-08-09  8:03   ` Christoph Hellwig

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=1438883072-28706-7-git-send-email-bfoster@redhat.com \
    --to=bfoster@redhat.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