All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: xfs <linux-xfs@vger.kernel.org>
Cc: Eric Sandeen <sandeen@redhat.com>
Subject: [PATCH 3/3] xfs: freeze rw filesystems just prior to reboot
Date: Wed, 17 May 2017 18:32:42 -0700	[thread overview]
Message-ID: <20170518013242.GW4519@birch.djwong.org> (raw)
In-Reply-To: <20170518012618.GT4519@birch.djwong.org>

Apparently there are certain system software configurations that do odd
things like update the kernel and reboot without umounting the /boot fs
or remounting it readonly, either of which would push all the AIL items
out to disk.  As a result, a subsequent invocation of something like
grub (which has a frightening willingness to read a fs with a dirty log)
can read stale disk contents and/or miss files the metadata for which
have been written to the log but not checkpointed into the filesystem.

Granted, most of the time /boot is a separate partition and
systemd/sysvinit/whatever actually /do/ unmount /boot before rebooting.
This "fix" is only needed for people who have one giant filesystem.

Therefore, add a reboot hook to freeze the rw filesystems (which
checkpoints the log) just prior to reboot.  This is an unfortunate and
insufficient workaround for multiple layers of inadequate external
software, but at least it will reduce boot time surprises for the "OS
updater failed to disengage the filesystem before rebooting" case.

Seeing as grub is unlikely ever to learn to replay the XFS log (and we
probably don't want it doing that), *LILO has been discontinued for at
least 18 months, and we're not quite to the point of putting kernel
files directly on the EFI System Partition, this seems like the least
crappy solution to this problem.

Yes, you're still screwed in grub if the system crashes. :)  I don't
anticipate this patch will make it upstream, but the idea could get at
least a single hearing.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 455a575..415b1e8 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -61,6 +61,7 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 #include <linux/parser.h>
+#include <linux/reboot.h>
 
 static const struct super_operations xfs_super_operations;
 struct bio_set *xfs_ioend_bioset;
@@ -1982,6 +1983,38 @@ xfs_destroy_workqueues(void)
 	destroy_workqueue(xfs_alloc_wq);
 }
 
+STATIC void
+xfs_reboot_fs(
+	struct super_block	*sb,
+	void			*priv)
+{
+	int			error;
+
+	if (sb->s_flags & MS_RDONLY)
+		return;
+	xfs_info(XFS_M(sb), "Freezing prior to reboot.");
+	up_read(&sb->s_umount);
+	error = freeze_super(sb);
+	down_read(&sb->s_umount);
+	if (error && error != -EBUSY)
+		xfs_info(XFS_M(sb), "Unable to freeze, error=%d", error);
+}
+
+STATIC int
+xfs_reboot(
+	struct notifier_block	*nb,
+	ulong			event,
+	void			*buf)
+{
+	iterate_supers_type(&xfs_fs_type, xfs_reboot_fs, NULL);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block xfs_reboot_notifier = {
+	.notifier_call = xfs_reboot,
+	.priority = INT_MAX,
+};
+
 STATIC int __init
 init_xfs_fs(void)
 {
@@ -2056,8 +2089,14 @@ init_xfs_fs(void)
 	error = register_filesystem(&xfs_fs_type);
 	if (error)
 		goto out_qm_exit;
+
+	error = register_reboot_notifier(&xfs_reboot_notifier);
+	if (error)
+		goto out_register_fs;
 	return 0;
 
+ out_register_fs:
+	unregister_filesystem(&xfs_fs_type);
  out_qm_exit:
 	xfs_qm_exit();
  out_remove_dbg_kobj:
@@ -2089,6 +2128,7 @@ init_xfs_fs(void)
 STATIC void __exit
 exit_xfs_fs(void)
 {
+	unregister_reboot_notifier(&xfs_reboot_notifier);
 	xfs_qm_exit();
 	unregister_filesystem(&xfs_fs_type);
 #ifdef DEBUG

  parent reply	other threads:[~2017-05-18  1:32 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-18  1:26 [RFCRAP 0/3?] xfs: OH GOD MY EYES! Darrick J. Wong
2017-05-18  1:30 ` [PATCH 1/3] xfs: remove double-underscore integer types Darrick J. Wong
2017-05-18  6:01   ` Dave Chinner
2017-05-18  6:21     ` Darrick J. Wong
2017-05-18  6:31     ` Christoph Hellwig
2017-05-18  1:31 ` [PATCH 2/3] xfsprogs: " Darrick J. Wong
2017-05-18  6:32   ` Christoph Hellwig
2017-05-23  2:48     ` Darrick J. Wong
2017-05-23  2:24   ` Eric Sandeen
2017-05-18  1:32 ` Darrick J. Wong [this message]
2017-05-18  6:28   ` [PATCH 3/3] xfs: freeze rw filesystems just prior to reboot Christoph Hellwig
2017-05-18  8:34   ` Dave Chinner
2017-05-18 22:30     ` Darrick J. Wong
2017-05-19 19:09       ` Chris Murphy
2017-05-19 21:00         ` Darrick J. Wong
2017-05-20  0:27           ` Chris Murphy
2017-05-22  2:07             ` Dave Chinner
     [not found]           ` <20170522020112.GV17542@dastard>
2017-05-22 20:46             ` Chris Murphy
2017-05-23  3:56               ` Chris Murphy
2017-05-23  4:04                 ` Eric Sandeen
2017-05-23 11:44                   ` Dave Chinner
2017-05-24  3:19               ` Dave Chinner
2017-05-24  8:06                 ` Chris Murphy
2017-05-24  6:22               ` Chris Murphy
2017-05-24  6:25                 ` Chris Murphy
2017-05-24 23:13                   ` Dave Chinner
2017-05-25  0:03                 ` Dave Chinner

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=20170518013242.GW4519@birch.djwong.org \
    --to=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.