e2fsck replays the journal only when the superblock's needs_recovery
flag is set, and ext2fs does not replay it itself. The flag was set
in diskfs_set_hypermetadata the first time the filesystem was dirtied,
which happens lazily, and the superblock then went through the
journal. So kjournald could commit transactions while the superblock
on disk still said that nothing needed recovery, until block 0 was
checkpointed or written by the pager. A crash in that window left
e2fsck with "needs_recovery flag is clear, but journal has data": with
-y it replays anyway, but in preen mode PR_0_JOURNAL_RUN is not
PR_PREEN_OK and the boot stops for a manual fsck. On clean shutdown,
the flag was cleared before the journal had been checkpointed and
emptied, and also on a goaway that then failed with EBUSY, leaving the
filesystem mounted and journaled with the flag clear.
Follow ext4: the flag is on disk whenever the log may hold
transactions, and is cleared only once the log on disk is empty.
ext2_set_needs_recovery sets it when the journal starts on a writable
filesystem and when such a filesystem becomes writable, before
anything is journaled. It reads the superblock from disk, sets only
the flag there, writes it back and flushes, outside the journal; the
rest of the in-memory superblock may hold changes that are not
committed yet. diskfs_journal_shutdown clears the flag the same way
with ext2_clear_needs_recovery, after quiescing has emptied the log and
the journal superblock is flushed, and leaves it set when quiescing
failed. diskfs_set_hypermetadata no longer touches it.
---
ext2fs/ext2fs.c | 4 ++
ext2fs/ext2fs.h | 6 +++
ext2fs/hyper.c | 110 +++++++++++++++++++++++++++++++++++++++++------
ext2fs/journal.c | 22 +++++++---
4 files changed, 122 insertions(+), 20 deletions(-)
diff --git a/ext2fs/ext2fs.c b/ext2fs/ext2fs.c
index 9bdaa2b0a..60af60f12 100644
--- a/ext2fs/ext2fs.c
+++ b/ext2fs/ext2fs.c
@@ -269,6 +269,10 @@ main (int argc, char **argv)
ext2_journal = journal_create (jnode);
if (ext2_journal)
fprintf (stderr, "ext2fs: journaling enabled on %s\n",
diskfs_disk_name);
+ /* Before anything is journaled. A read-only filesystem gets
+ the flag when it becomes writable. */
+ if (ext2_journal && !diskfs_readonly)
+ ext2_set_needs_recovery ();
JRNL_LOG_DEBUG ("Global Journal Initialized at %p", ext2_journal);
diskfs_nput(jnode);
}
diff --git a/ext2fs/ext2fs.h b/ext2fs/ext2fs.h
index 8aabc6434..98e3d952a 100644
--- a/ext2fs/ext2fs.h
+++ b/ext2fs/ext2fs.h
@@ -381,6 +381,12 @@ diskfs_transaction_t *
journal_thread_transaction (void);
void ext2_orphan_drop_ram_link (struct node *np);
+
+/* Set the needs_recovery flag directly on disk, outside the journal. */
+void ext2_set_needs_recovery (void);
+
+/* Clear the needs_recovery flag once the journal is empty on disk. */
+void ext2_clear_needs_recovery (void);
/* ---------------------------------------------------------------- */
/* Random stuff calculated from the super block. */
diff --git a/ext2fs/hyper.c b/ext2fs/hyper.c
index 60be3e22f..5035de819 100644
--- a/ext2fs/hyper.c
+++ b/ext2fs/hyper.c
@@ -196,14 +196,10 @@ diskfs_set_hypermetadata (int wait, int clean)
error_t err = 0;
diskfs_transaction_t *txn = diskfs_journal_start_transaction ();
+ /* The needs_recovery flag is not handled here: see
+ ext2_set_needs_recovery and ext2_clear_needs_recovery. */
if (clean)
{
- /* Always clear recovery flag on clean unmount if journal is present */
- if (ext2_journal && (sblock->s_feature_incompat &
htole32(EXT3_FEATURE_INCOMPAT_RECOVER)))
- {
- sblock->s_feature_incompat &=
htole32(~EXT3_FEATURE_INCOMPAT_RECOVER);
- sblock_dirty = 1;
- }
/* Only set EXT2_VALID_FS if it was clean when we mounted it */
if (ext2fs_clean && !(sblock->s_state & htole16 (EXT2_VALID_FS)))
{
@@ -213,13 +209,6 @@ diskfs_set_hypermetadata (int wait, int clean)
}
else
{
- /* Always set recovery flag when dirtying if journal is present */
- if (ext2_journal && !(sblock->s_feature_incompat &
htole32(EXT3_FEATURE_INCOMPAT_RECOVER)))
- {
- sblock->s_feature_incompat |= htole32(EXT3_FEATURE_INCOMPAT_RECOVER);
- sblock_dirty = 1;
- wait = 1;
- }
/* Clear EXT2_VALID_FS */
if (sblock->s_state & htole16 (EXT2_VALID_FS))
{
@@ -266,6 +255,98 @@ out:
return err;
}
+/* Set (SET nonzero) or clear the needs_recovery flag in SBLOCK, in the
+ cached superblock, and in the superblock on disk, and flush. Only the
+ flag is changed on disk, outside the journal: the rest of SBLOCK may
+ hold changes that are not committed yet, and diskfs_set_hypermetadata
+ cannot write while the filesystem is going read-only. */
+static error_t
+write_needs_recovery_flag (int set)
+{
+ uint32_t flag = htole32 (EXT3_FEATURE_INCOMPAT_RECOVER);
+ struct ext2_super_block *disk_sblock = NULL;
+ size_t amount = 0;
+ error_t err;
+
+ if (set)
+ {
+ sblock->s_feature_incompat |= flag;
+ /* Later writes of the superblock's block come from this image. */
+ mapped_sblock->s_feature_incompat |= flag;
+ }
+ else
+ {
+ sblock->s_feature_incompat &= ~flag;
+ mapped_sblock->s_feature_incompat &= ~flag;
+ }
+
+ err = store_read (store, SBLOCK_OFFS >> store->log2_block_size,
+ SBLOCK_SIZE, (void **) &disk_sblock, &amount);
+ if (!err && amount != SBLOCK_SIZE)
+ err = EIO;
+ if (!err)
+ {
+ if (set)
+ disk_sblock->s_feature_incompat |= flag;
+ else
+ disk_sblock->s_feature_incompat &= ~flag;
+ err = store_write (store, SBLOCK_OFFS >> store->log2_block_size,
+ disk_sblock, SBLOCK_SIZE, &amount);
+ if (!err && amount != SBLOCK_SIZE)
+ err = EIO;
+ }
+ if (disk_sblock)
+ munmap (disk_sblock, SBLOCK_SIZE);
+ if (!err)
+ {
+ err = store_sync (store);
+ if (err == EOPNOTSUPP || err == D_INVALID_OPERATION)
+ err = 0;
+ }
+ return err;
+}
+
+/* Set the needs_recovery flag on disk, outside the journal, and flush.
+ e2fsck replays the log only when the flag is set, and ext2fs does not
+ replay it itself, so the flag must be on disk before the journal
+ commits anything, and must not wait for the journal to write the
+ superblock home. Called when the journal starts on a writable
+ filesystem, or when such a filesystem becomes writable, before any
+ transaction holds the superblock. The flag stays set until the journal
+ is empty on disk; see ext2_clear_needs_recovery. */
+void
+ext2_set_needs_recovery (void)
+{
+ error_t err;
+
+ if (sblock->s_feature_incompat & htole32 (EXT3_FEATURE_INCOMPAT_RECOVER))
+ return;
+
+ err = write_needs_recovery_flag (1);
+ if (err)
+ ext2_warning ("cannot set the needs_recovery flag on disk: %s; "
+ "after a crash, run e2fsck to replay the journal",
+ strerror (err));
+}
+
+/* Clear the needs_recovery flag on disk. Called once the journal is
+ stopped and empty on disk. */
+void
+ext2_clear_needs_recovery (void)
+{
+ error_t err;
+
+ assert_backtrace (!ext2_journal);
+
+ if (!(sblock->s_feature_incompat & htole32 (EXT3_FEATURE_INCOMPAT_RECOVER)))
+ return;
+
+ err = write_needs_recovery_flag (0);
+ if (err)
+ ext2_warning ("cannot clear the needs_recovery flag on disk: %s",
+ strerror (err));
+}
+
void
diskfs_readonly_changed (int readonly)
{
@@ -278,4 +359,7 @@ diskfs_readonly_changed (int readonly)
if (!readonly && !(sblock->s_state & htole16 (EXT2_VALID_FS)))
ext2_warning ("UNCLEANED FILESYSTEM NOW WRITABLE");
+
+ if (!readonly && ext2_journal)
+ ext2_set_needs_recovery ();
}
diff --git a/ext2fs/journal.c b/ext2fs/journal.c
index d85e1374c..fcff52c52 100644
--- a/ext2fs/journal.c
+++ b/ext2fs/journal.c
@@ -1595,7 +1595,7 @@ journal_create_running_transaction_locked (journal_t
*journal)
* Forcefully clears the checkpoint list.
* SAFE ONLY after a full filesystem sync.
*/
-static void
+static error_t
journal_clear_checkpoint_list_locked (journal_t *journal)
{
diskfs_transaction_t *txn = journal->j_checkpoint_list;
@@ -1619,6 +1619,7 @@ journal_clear_checkpoint_list_locked (journal_t *journal)
err = journal_update_superblock (journal, journal->j_transaction_sequence);
if (err)
JRNL_LOG_WARN ("Failed to update superblock. %s", strerror (err));
+ return err;
}
/**
@@ -2998,8 +2999,9 @@ journal_create (struct node *journal_inode)
* Safely marks the journal as clean on disk.
* MUST only be called after sync_global(1) ensures no pager I/O is in flight,
* otherwise asynchronous pager notifications will cause a Use-After-Free!
+ * Returns 1 if the journal superblock on disk now has an empty log.
*/
-static void
+static int
journal_quiesce_checkpoints (void)
{
JRNL_LOG_DEBUG ("Journal in quiesce checkpoints.");
@@ -3025,7 +3027,7 @@ journal_quiesce_checkpoints (void)
JRNL_LOG_WARN
("Quiesce timed out! Transaction deadlocked. Leaving journal dirty.");
JOURNAL_UNLOCK (ext2_journal);
- return;
+ return 0;
}
/* Write ALL shadow data from every checkpoint transaction to the main disk.
*/
@@ -3038,13 +3040,14 @@ journal_quiesce_checkpoints (void)
JRNL_LOG_WARN ("[QUIESCE] I/O error during final checkpoint. "
"Leaving journal dirty for WAL replay.");
JOURNAL_UNLOCK (ext2_journal);
- return;
+ return 0;
}
/* Now safe to clear the list and write s_start = 0 to the JBD2 superblock */
- journal_clear_checkpoint_list_locked (ext2_journal);
+ err = journal_clear_checkpoint_list_locked (ext2_journal);
JOURNAL_UNLOCK (ext2_journal);
+ return !err;
}
static void
@@ -3540,12 +3543,17 @@ diskfs_journal_shutdown (void)
sync_global (1);
/* Checkpoint all remaining transactions and mark the journal clean. */
- journal_quiesce_checkpoints ();
+ int empty = journal_quiesce_checkpoints ();
ext2_journal = NULL;
- /* Final hardware flush. */
+ /* Final hardware flush. It also puts the empty journal superblock on
+ disk before the needs_recovery flag is cleared. */
error_t err = store_sync (store);
if (err && err != EOPNOTSUPP && err != D_INVALID_OPERATION)
ext2_warning ("device flush failed: %s", strerror (err));
+
+ /* Left set when the log is not empty, so that e2fsck replays it. */
+ if (empty)
+ ext2_clear_needs_recovery ();
}
--
2.56.0
