GFS2: mark the journal idle to fix ro mounts

When gfs2 was mounted read-only and then unmounted, it was writing a header block to the journal in the syncing gfs2_log_flush() call from kill_sb(). This is because the journal was not being marked as idle until the first log header was written out, and on a read-only mount there never was a log header written out. Since the journal was not marked idle, gfs2_log_flush() was writing out a header lock to make sure it was empty during the sync. Not only did this cause IO to a read-only filesystem, but the journalling isn't completely initialized on read-only mounts, and so gfs2 was writing out the wrong sequence number in the log header.

Now, the journal is marked idle on mount, and gfs2_log_flush() won't write out anything until there starts being transactions to flush.

Signed-off-by Benjamin Marzinski <[email protected]>

When gfs2 was mounted read-only and then unmounted, it was writing a header block to the journal in the syncing gfs2_log_flush() call from kill_sb(). This is because the journal was not being marked as idle until the first log header was written out, and on a read-only mount there never was a log header written out. Since the journal was not marked idle, gfs2_log_flush() was writing out a header lock to make sure it was empty during the sync. Not only did this cause IO to a read-only filesystem, but the journalling isn't completely initialized on read-only mounts, and so gfs2 was writing out the wrong sequence number in the log header.

Now, the journal is marked idle on mount, and gfs2_log_flush() won't write out anything until there starts being transactions to flush.

Signed-off-by Benjamin Marzinski <[email protected]>

---
  fs/gfs2/ops_fstype.c | 1 +
  1 file changed, 1 insertion(+)

fs/gfs2/ops_fstype.c +1 -0

index 8633ad3..fd984f6 100644
@@ -757,6 +757,7 @@ static int init_journal(struct gfs2_sbd *sdp, int undo)
  		}
  	}
 
+	sdp->sd_log_idle = 1;
  	set_bit(SDF_JOURNAL_CHECKED, &sdp->sd_flags);
  	gfs2_glock_dq_uninit(&ji_gh);
  	jindex = 0;

Hi,

Quoted 11 lines · Benjamin Marzinski

When gfs2 was mounted read-only and then unmounted, it was writing a header block to the journal in the syncing gfs2_log_flush() call from kill_sb(). This is because the journal was not being marked as idle until the first log header was written out, and on a read-only mount there never was a log header written out. Since the journal was not marked idle, gfs2_log_flush() was writing out a header lock to make sure it was empty during the sync. Not only did this cause IO to a read-only filesystem, but the journalling isn't completely initialized on read-only mounts, and so gfs2 was writing out the wrong sequence number in the log header.

Quoted 2 lines

Now, the journal is marked idle on mount, and gfs2_log_flush() won't write out anything until there starts being transactions to flush.

Does that mean that we should be doing more to initialize the log in the r/o mount case? It should know enough to recover the journals in the case that it is the first mounter, so did this perhaps only apply to subsequent mounters of the filesystem?

Still it is a good catch!

Acked-by Steven Whitehouse <[email protected]>

Steve.

Quoted 16 lines

Signed-off-by Benjamin Marzinski <[email protected]>

---
 fs/gfs2/ops_fstype.c | 1 +
 1 file changed, 1 insertion(+)

fs/gfs2/ops_fstype.c +1 -0

index 8633ad3..fd984f6 100644
@@ -757,6 +757,7 @@ static int init_journal(struct gfs2_sbd *sdp, int undo)
 		}
 	}
+	sdp->sd_log_idle = 1;
 	set_bit(SDF_JOURNAL_CHECKED, &sdp->sd_flags);
 	gfs2_glock_dq_uninit(&ji_gh);
 	jindex = 0;

Quoted 14 lines · Steven Whitehouse

Hi,

Quoted 11 lines · Benjamin Marzinski

When gfs2 was mounted read-only and then unmounted, it was writing a header block to the journal in the syncing gfs2_log_flush() call from kill_sb(). This is because the journal was not being marked as idle until the first log header was written out, and on a read-only mount there never was a log header written out. Since the journal was not marked idle, gfs2_log_flush() was writing out a header lock to make sure it was empty during the sync. Not only did this cause IO to a read-only filesystem, but the journalling isn't completely initialized on read-only mounts, and so gfs2 was writing out the wrong sequence number in the log header.

Quoted 6 lines

Quoted 2 lines

Now, the journal is marked idle on mount, and gfs2_log_flush() won't write out anything until there starts being transactions to flush.

Does that mean that we should be doing more to initialize the log in the r/o mount case? It should know enough to recover the journals in the case that it is the first mounter, so did this perhaps only apply to subsequent mounters of the filesystem?

gfs2 currently has enough information to do recovery. Both gfs2_recover_func() and gfs2_make_fs_rw() call gfs2_find_jhead() to get information about the head of the journal and the sequence numbers.

gfs2_make_fs_rw() saves this information with these lines

         /*  Initialize some head of the log stuff  */
         sdp->sd_log_sequence = head.lh_sequence + 1;
         gfs2_log_pointers_init(sdp, head.lh_blkno);

This is what's not getting called on the read-only mounts that was causing the fsck error. But the read only mounts should never be writing anything in gfs2_log_flush(), which is where these values are used. So we could make sure these values are always initialized, but the fact that they weren't was what allowed us to catch this bug (and it would still be a bug to write that header, even if it didn't mess with fsck).

-Ben

When gfs2 was mounted read-only and then unmounted, it was writing a header block to the journal in the syncing gfs2_log_flush() call from kill_sb(). This is because the journal was not being marked as idle until the first log header was written out, and on a read-only mount there never was a log header written out. Since the journal was not marked idle, gfs2_log_flush() was writing out a header lock to make sure it was empty during the sync. Not only did this cause IO to a read-only filesystem, but the journalling isn't completely initialized on read-only mounts, and so gfs2 was writing out the wrong sequence number in the log header.

Now, the journal is marked idle on mount, and gfs2_log_flush() won't write out anything until there starts being transactions to flush.

Signed-off-by Benjamin Marzinski <[email protected]>

---
 fs/gfs2/ops_fstype.c | 1 +
 1 file changed, 1 insertion(+)

fs/gfs2/ops_fstype.c +1 -1

index 8633ad3..fd984f6 100644
@@ -757,6 +757,7 @@ static int init_journal(struct gfs2_sbd *sdp, int undo)
 		}
 	}
 
+	sdp->sd_log_idle = 1;
 	set_bit(SDF_JOURNAL_CHECKED, &sdp->sd_flags);
 	gfs2_glock_dq_uninit(&ji_gh);
 	jindex = 0;
--

1.8.3.1