iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

gfs2: move freeze glock outside the make_fs_rw and _ro functions

Browse Source 
Before this patch, sister functions gfs2_make_fs_rw and gfs2_make_fs_ro locked
(held) the freeze glock by calling gfs2_freeze_lock and gfs2_freeze_unlock.
The problem is, not all the callers of gfs2_make_fs_ro should be doing this.
The three callers of gfs2_make_fs_ro are: remount (gfs2_reconfigure),
signal_our_withdraw, and unmount (gfs2_put_super). But when unmounting the
file system we can get into the following circular lock dependency:

deactivate_super
   down_write(&s->s_umount); <-------------------------------------- s_umount
   deactivate_locked_super
      gfs2_kill_sb
         kill_block_super
            generic_shutdown_super
               gfs2_put_super
                  gfs2_make_fs_ro
                     gfs2_glock_nq_init sd_freeze_gl
                        freeze_go_sync
                           if (freeze glock in SH)
                              freeze_super (vfs)
                                 down_write(&sb->s_umount); <------- s_umount

This patch moves the hold of the freeze glock outside the two sister rw/ro
functions to their callers, but it doesn't request the glock from
gfs2_put_super, thus eliminating the circular dependency.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
This commit is contained in:
Bob Peterson 2020-12-22 14:43:28 -06:00 committed by Andreas Gruenbacher
parent c77b52c0a1
commit 96b1454f2e
3 changed files with 33 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
fs/gfs2/ops_fstype.c
View File

@ -1084,6 +1084,7 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
int silent = fc->sb_flags & SB_SILENT;
struct gfs2_sbd *sdp;
struct gfs2_holder mount_gh;
struct gfs2_holder freeze_gh;
int error;
sdp = init_sbd(sb);
@ -1195,23 +1196,18 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
goto fail_per_node;
}
if (sb_rdonly(sb)) {
struct gfs2_holder freeze_gh;
error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
if (error)
goto fail_per_node;
error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
if (error) {
fs_err(sdp, "can't make FS RO: %d\n", error);
goto fail_per_node;
}
gfs2_freeze_unlock(&freeze_gh);
} else {
if (!sb_rdonly(sb))
error = gfs2_make_fs_rw(sdp);
if (error) {
fs_err(sdp, "can't make FS RW: %d\n", error);
goto fail_per_node;
}
}
gfs2_freeze_unlock(&freeze_gh);
if (error) {
fs_err(sdp, "can't make FS RW: %d\n", error);
goto fail_per_node;
}
gfs2_glock_dq_uninit(&mount_gh);
gfs2_online_uevent(sdp);
return 0;
@ -1512,6 +1508,12 @@ static int gfs2_reconfigure(struct fs_context *fc)
fc->sb_flags |= SB_RDONLY;
if ((sb->s_flags ^ fc->sb_flags) & SB_RDONLY) {
struct gfs2_holder freeze_gh;
error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
if (error)
return -EINVAL;
if (fc->sb_flags & SB_RDONLY) {
error = gfs2_make_fs_ro(sdp);
if (error)
@ -1521,6 +1523,7 @@ static int gfs2_reconfigure(struct fs_context *fc)
if (error)
errorfc(fc, "unable to remount read-write");
}
gfs2_freeze_unlock(&freeze_gh);
}
sdp->sd_args = *newargs;

23
fs/gfs2/super.c
View File

@ -165,7 +165,6 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
{
struct gfs2_inode *ip = GFS2_I(sdp->sd_jdesc->jd_inode);
struct gfs2_glock *j_gl = ip->i_gl;
struct gfs2_holder freeze_gh;
struct gfs2_log_header_host head;
int error;
@ -173,10 +172,6 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
if (error)
return error;
error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
if (error)
goto fail_threads;
j_gl->gl_ops->go_inval(j_gl, DIO_METADATA);
if (gfs2_withdrawn(sdp)) {
error = -EIO;
@ -203,13 +198,9 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
gfs2_freeze_unlock(&freeze_gh);
return 0;
fail:
gfs2_freeze_unlock(&freeze_gh);
fail_threads:
if (sdp->sd_quotad_process)
kthread_stop(sdp->sd_quotad_process);
sdp->sd_quotad_process = NULL;
@ -607,21 +598,9 @@ out:
int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
{
struct gfs2_holder freeze_gh;
int error = 0;
int log_write_allowed = test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
gfs2_holder_mark_uninitialized(&freeze_gh);
if (sdp->sd_freeze_gl &&
!gfs2_glock_is_locked_by_me(sdp->sd_freeze_gl)) {
error = gfs2_freeze_lock(sdp, &freeze_gh,
log_write_allowed ? 0 : LM_FLAG_TRY);
if (error == GLR_TRYFAILED)
error = 0;
if (error && !gfs2_withdrawn(sdp))
return error;
}
gfs2_flush_delete_work(sdp);
if (!log_write_allowed && current == sdp->sd_quotad_process)
fs_warn(sdp, "The quotad daemon is withdrawing.\n");
@ -650,8 +629,6 @@ int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
atomic_read(&sdp->sd_reserving_log) == 0,
HZ * 5);
}
gfs2_freeze_unlock(&freeze_gh);
gfs2_quota_cleanup(sdp);
if (!log_write_allowed)

18
fs/gfs2/util.c
View File

@ -122,6 +122,7 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
struct inode *inode = sdp->sd_jdesc->jd_inode;
struct gfs2_inode *ip = GFS2_I(inode);
u64 no_formal_ino = ip->i_no_formal_ino;
int log_write_allowed = test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
int ret = 0;
int tries;
@ -142,8 +143,21 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
* therefore we need to clear SDF_JOURNAL_LIVE manually.
*/
clear_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
if (!sb_rdonly(sdp->sd_vfs))
ret = gfs2_make_fs_ro(sdp);
if (!sb_rdonly(sdp->sd_vfs)) {
struct gfs2_holder freeze_gh;
gfs2_holder_mark_uninitialized(&freeze_gh);
if (sdp->sd_freeze_gl &&
!gfs2_glock_is_locked_by_me(sdp->sd_freeze_gl)) {
ret = gfs2_freeze_lock(sdp, &freeze_gh,
log_write_allowed ? 0 : LM_FLAG_TRY);
if (ret == GLR_TRYFAILED)
ret = 0;
}
if (!ret)
ret = gfs2_make_fs_ro(sdp);
gfs2_freeze_unlock(&freeze_gh);
}
if (sdp->sd_lockstruct.ls_ops->lm_lock == NULL) { /* lock_nolock */
if (!ret)