gfs2: clear journal live bit in gfs2_log_flush
PATCH 1/1 gfs2: clear journal live bit in gfs2_log_flush
When gfs2 was unmounting filesystems or changing them to read-only it was clearing the SDF_JOURNAL_LIVE bit before the final log flush. This caused a race. If an inode glock got demoted in the gap between clearing the bit and the shutdown flush, it would be unable to reserve log space to clear out the acive items list in inode_go_sync, causing an error in inode_go_inval because the glock was still dirty.
To solve this, the SDF_JOURNAL_LIVE bit is now cleared inside the shutdown log flush. This means that, because of the locking on the log blocks, either inode_go_sync will be able to reserve space to clean the glock before the shutdown flush, or the shutdown flush will clean the glock itself, before inode_go_sync fails to reserve the space. Either way, the glock will be clean before inode_go_inval.
--- fs/gfs2/log.c | 3 +++ fs/gfs2/super.c | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-)
fs/gfs2/log.c +3 -0
@@ -716,6 +716,9 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, }trace_gfs2_log_flush (sdp,1 ); +if (type ==SHUTDOWN_FLUSH ) +clear_bit (SDF_JOURNAL_LIVE , &sdp->sd_flags); + sdp->sd_log_flush_head = sdp->sd_log_head; sdp->sd_log_flush_wrapped =0 ; tr = sdp->sd_log_tr;
fs/gfs2/super.c +0 -4
@@ -842,10 +842,6 @@ static int gfs2_make_fs_ro(struct gfs2_sbd *sdp)gfs2_quota_sync (sdp->sd_vfs,0 );gfs2_statfs_sync (sdp->sd_vfs,0 ); -down_write (&sdp->sd_log_flush_lock); -clear_bit (SDF_JOURNAL_LIVE , &sdp->sd_flags); -up_write (&sdp->sd_log_flush_lock); -gfs2_log_flush (sdp,NULL ,SHUTDOWN_FLUSH );wait_event (sdp->sd_reserving_log_wait,atomic_read (&sdp->sd_reserving_log) ==0 );gfs2_assert_warn (sdp,atomic_read (&sdp->sd_log_blks_free) == sdp->sd_jdesc->jd_blocks);
Signature · 1 line
1.8.3.1
PATCH 1/1 gfs2: clear journal live bit in gfs2_log_flush
----- Original Message -----
Quoted 53 lines
When gfs2 was unmounting filesystems or changing them to read-only it was clearing the
SDF_JOURNAL_LIVEbit before the final log flush. This caused a race. If an inode glock got demoted in the gap between clearing the bit and the shutdown flush, it would be unable to reserve log space to clear out the acive items list ininode_go_sync, causing an error ininode_go_invalbecause the glock was still dirty.To solve this, the
SDF_JOURNAL_LIVEbit is now cleared inside the shutdown log flush. This means that, because of the locking on the log blocks, eitherinode_go_syncwill be able to reserve space to clean the glock before the shutdown flush, or the shutdown flush will clean the glock itself, beforeinode_go_syncfails to reserve the space. Either way, the glock will be clean beforeinode_go_inval.Signed-off-by Benjamin Marzinski <[email protected]>--- fs/gfs2/log.c | 3 +++ fs/gfs2/super.c | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-)fs/gfs2/log.c +3
-0@@ -716,6 +716,9 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, }trace_gfs2_log_flush (sdp,1 ); +if (type ==SHUTDOWN_FLUSH ) +clear_bit (SDF_JOURNAL_LIVE , &sdp->sd_flags); + sdp->sd_log_flush_head = sdp->sd_log_head; sdp->sd_log_flush_wrapped =0 ;fs/gfs2/log.c +0
-0tr = sdp->sd_log_tr;fs/gfs2/super.c +0
-4@@ -842,10 +842,6 @@ static int gfs2_make_fs_ro(struct gfs2_sbd *sdp)gfs2_quota_sync (sdp->sd_vfs,0 );gfs2_statfs_sync (sdp->sd_vfs,0 ); -down_write (&sdp->sd_log_flush_lock); -clear_bit (SDF_JOURNAL_LIVE , &sdp->sd_flags); -up_write (&sdp->sd_log_flush_lock); -gfs2_log_flush (sdp,NULL ,SHUTDOWN_FLUSH );wait_event (sdp->sd_reserving_log_wait,atomic_read (&sdp->sd_reserving_log) ==0 );fs/gfs2/super.c +0
-1gfs2_assert_warn (sdp,atomic_read (&sdp->sd_log_blks_free) == sdp->sd_jdesc->jd_blocks); --
Quoted 3 lines
1.8.3.1
Hi,
Thanks. This is now applied to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/cgit/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=3ceb22a7b7a1e50658cce8c43d942e9f31f654bd
Regards,
Bob Peterson
Red Hat File Systems
Andreas Gruenbacher
maintainerPATCH 1/1 gfs2: clear journal live bit in gfs2_log_flush
In reply to Benjamin MarzinskiBen,
the fix is looking good and I can no longer trigger this bug, thanks.
Andreas
PATCH 1/1 gfs2: clear journal live bit in gfs2_log_flush
In reply to Benjamin MarzinskiHi,
Looks good to me. We should really add the "type" into the log flush tracing though. I'd quite like to also be able to differentiate between log flushes caused by timeouts, and those caused by glock dropping (and possibly other causes too) but I know that is a much more tricky thing to do. It would be very handy for performance analysis though,
Steve.
Quoted 49 lines · Benjamin Marzinski
When gfs2 was unmounting filesystems or changing them to read-only it was clearing the
SDF_JOURNAL_LIVEbit before the final log flush. This caused a race. If an inode glock got demoted in the gap between clearing the bit and the shutdown flush, it would be unable to reserve log space to clear out the acive items list ininode_go_sync, causing an error ininode_go_invalbecause the glock was still dirty.To solve this, the
SDF_JOURNAL_LIVEbit is now cleared inside the shutdown log flush. This means that, because of the locking on the log blocks, eitherinode_go_syncwill be able to reserve space to clean the glock before the shutdown flush, or the shutdown flush will clean the glock itself, beforeinode_go_syncfails to reserve the space. Either way, the glock will be clean beforeinode_go_inval.Signed-off-by Benjamin Marzinski <[email protected]>--- fs/gfs2/log.c | 3 +++ fs/gfs2/super.c | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-)fs/gfs2/log.c +3
-0@@ -716,6 +716,9 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, }trace_gfs2_log_flush (sdp,1 ); +if (type ==SHUTDOWN_FLUSH ) +clear_bit (SDF_JOURNAL_LIVE , &sdp->sd_flags); + sdp->sd_log_flush_head = sdp->sd_log_head; sdp->sd_log_flush_wrapped =0 ; tr = sdp->sd_log_tr;fs/gfs2/super.c +0
-4@@ -842,10 +842,6 @@ static int gfs2_make_fs_ro(struct gfs2_sbd *sdp)gfs2_quota_sync (sdp->sd_vfs,0 );gfs2_statfs_sync (sdp->sd_vfs,0 ); -down_write (&sdp->sd_log_flush_lock); -clear_bit (SDF_JOURNAL_LIVE , &sdp->sd_flags); -up_write (&sdp->sd_log_flush_lock); -gfs2_log_flush (sdp,NULL ,SHUTDOWN_FLUSH );wait_event (sdp->sd_reserving_log_wait,atomic_read (&sdp->sd_reserving_log) ==0 );gfs2_assert_warn (sdp,atomic_read (&sdp->sd_log_blks_free) == sdp->sd_jdesc->jd_blocks);