* [Cluster-devel] [GFS2 v2 PATCH 0/2] Circular lock dependency on unmount
@ 2020-12-22 20:43 Bob Peterson
2020-12-22 20:43 ` [Cluster-devel] [GFS2 PATCH 1/2] gfs2: Add common helper for holding and releasing the freeze glock Bob Peterson
2020-12-22 20:43 ` [Cluster-devel] [GFS2 PATCH 2/2] gfs2: move freeze glock outside the make_fs_rw and _ro functions Bob Peterson
0 siblings, 2 replies; 4+ messages in thread
From: Bob Peterson @ 2020-12-22 20:43 UTC (permalink / raw)
To: cluster-devel.redhat.com
Version 2 implements Andreas's suggestions.
This patch set fixes a circular lock dependency flagged by LOCKDEP when a
gfs2 file system is unmounted. The first patch adds new helper functions
for holding and releasing the freeze glock. Functionality should be the same.
The second patch moves the freeze locks that used to be inside the
make_fs_ro and _rw functions to their callers. The unmount caller, however,
is exempted to avoid the lock dependency. The change means that function
init_threads is now called from within the freeze lock, but that should
not matter.
Bob Peterson (2):
gfs2: Add common helper for holding and releasing the freeze glock
gfs2: move freeze glock outside the make_fs_rw and _ro functions
fs/gfs2/ops_fstype.c | 33 ++++++++++++++++----------------
fs/gfs2/recovery.c | 8 +++-----
fs/gfs2/super.c | 45 ++++----------------------------------------
fs/gfs2/util.c | 43 ++++++++++++++++++++++++++++++++++++++++--
fs/gfs2/util.h | 3 +++
5 files changed, 68 insertions(+), 64 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH 1/2] gfs2: Add common helper for holding and releasing the freeze glock
2020-12-22 20:43 [Cluster-devel] [GFS2 v2 PATCH 0/2] Circular lock dependency on unmount Bob Peterson
@ 2020-12-22 20:43 ` Bob Peterson
2020-12-22 20:43 ` [Cluster-devel] [GFS2 PATCH 2/2] gfs2: move freeze glock outside the make_fs_rw and _ro functions Bob Peterson
1 sibling, 0 replies; 4+ messages in thread
From: Bob Peterson @ 2020-12-22 20:43 UTC (permalink / raw)
To: cluster-devel.redhat.com
Many places in the gfs2 code queued and dequeued the freeze glock.
Almost all of them acquire it in SHARED mode, and need to specify the
same LM_FLAG_NOEXP and GL_EXACT flags.
This patch adds common helper functions gfs2_freeze_lock and gfs2_freeze_unlock
to make the code more readable, and to prepare for the next patch.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/ops_fstype.c | 6 ++----
fs/gfs2/recovery.c | 8 +++-----
fs/gfs2/super.c | 42 ++++++++++++++----------------------------
fs/gfs2/util.c | 25 +++++++++++++++++++++++++
fs/gfs2/util.h | 3 +++
5 files changed, 47 insertions(+), 37 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 3d9a6d6d42cb..c71c7beb9257 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1198,14 +1198,12 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
if (sb_rdonly(sb)) {
struct gfs2_holder freeze_gh;
- error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED,
- LM_FLAG_NOEXP | GL_EXACT,
- &freeze_gh);
+ 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_glock_dq_uninit(&freeze_gh);
+ gfs2_freeze_unlock(&freeze_gh);
} else {
error = gfs2_make_fs_rw(sdp);
if (error) {
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index cd3e66cdb560..b32db50049cd 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -469,9 +469,7 @@ static void gfs2_recover_one(struct gfs2_jdesc *jd)
/* Acquire a shared hold on the freeze lock */
- error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED,
- LM_FLAG_NOEXP | LM_FLAG_PRIORITY |
- GL_EXACT, &thaw_gh);
+ error = gfs2_freeze_lock(sdp, &thaw_gh, LM_FLAG_PRIORITY);
if (error)
goto fail_gunlock_ji;
@@ -521,7 +519,7 @@ static void gfs2_recover_one(struct gfs2_jdesc *jd)
clean_journal(jd, &head);
up_read(&sdp->sd_log_flush_lock);
- gfs2_glock_dq_uninit(&thaw_gh);
+ gfs2_freeze_unlock(&thaw_gh);
t_rep = ktime_get();
fs_info(sdp, "jid=%u: Journal replayed in %lldms [jlck:%lldms, "
"jhead:%lldms, tlck:%lldms, replay:%lldms]\n",
@@ -543,7 +541,7 @@ static void gfs2_recover_one(struct gfs2_jdesc *jd)
goto done;
fail_gunlock_thaw:
- gfs2_glock_dq_uninit(&thaw_gh);
+ gfs2_freeze_unlock(&thaw_gh);
fail_gunlock_ji:
if (jlocked) {
gfs2_glock_dq_uninit(&ji_gh);
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 2f56acc41c04..ea312a94ce69 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -173,9 +173,7 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
if (error)
return error;
- error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED,
- LM_FLAG_NOEXP | GL_EXACT,
- &freeze_gh);
+ error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
if (error)
goto fail_threads;
@@ -205,12 +203,12 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
- gfs2_glock_dq_uninit(&freeze_gh);
+ gfs2_freeze_unlock(&freeze_gh);
return 0;
fail:
- gfs2_glock_dq_uninit(&freeze_gh);
+ gfs2_freeze_unlock(&freeze_gh);
fail_threads:
if (sdp->sd_quotad_process)
kthread_stop(sdp->sd_quotad_process);
@@ -452,7 +450,7 @@ static int gfs2_lock_fs_check_clean(struct gfs2_sbd *sdp)
}
if (error)
- gfs2_glock_dq_uninit(&sdp->sd_freeze_gh);
+ gfs2_freeze_unlock(&sdp->sd_freeze_gh);
out:
while (!list_empty(&list)) {
@@ -616,21 +614,12 @@ int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
gfs2_holder_mark_uninitialized(&freeze_gh);
if (sdp->sd_freeze_gl &&
!gfs2_glock_is_locked_by_me(sdp->sd_freeze_gl)) {
- if (!log_write_allowed) {
- error = gfs2_glock_nq_init(sdp->sd_freeze_gl,
- LM_ST_SHARED, LM_FLAG_TRY |
- LM_FLAG_NOEXP | GL_EXACT,
- &freeze_gh);
- if (error == GLR_TRYFAILED)
- error = 0;
- } else {
- error = gfs2_glock_nq_init(sdp->sd_freeze_gl,
- LM_ST_SHARED,
- LM_FLAG_NOEXP | GL_EXACT,
- &freeze_gh);
- if (error && !gfs2_withdrawn(sdp))
- return error;
- }
+ 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);
@@ -661,8 +650,7 @@ int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
atomic_read(&sdp->sd_reserving_log) == 0,
HZ * 5);
}
- if (gfs2_holder_initialized(&freeze_gh))
- gfs2_glock_dq_uninit(&freeze_gh);
+ gfs2_freeze_unlock(&freeze_gh);
gfs2_quota_cleanup(sdp);
@@ -772,10 +760,8 @@ void gfs2_freeze_func(struct work_struct *work)
struct super_block *sb = sdp->sd_vfs;
atomic_inc(&sb->s_active);
- error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED,
- LM_FLAG_NOEXP | GL_EXACT, &freeze_gh);
+ error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
if (error) {
- fs_info(sdp, "GFS2: couldn't get freeze lock : %d\n", error);
gfs2_assert_withdraw(sdp, 0);
} else {
atomic_set(&sdp->sd_freeze_state, SFS_UNFROZEN);
@@ -785,7 +771,7 @@ void gfs2_freeze_func(struct work_struct *work)
error);
gfs2_assert_withdraw(sdp, 0);
}
- gfs2_glock_dq_uninit(&freeze_gh);
+ gfs2_freeze_unlock(&freeze_gh);
}
deactivate_super(sb);
clear_bit_unlock(SDF_FS_FROZEN, &sdp->sd_flags);
@@ -853,7 +839,7 @@ static int gfs2_unfreeze(struct super_block *sb)
return 0;
}
- gfs2_glock_dq_uninit(&sdp->sd_freeze_gh);
+ gfs2_freeze_unlock(&sdp->sd_freeze_gh);
mutex_unlock(&sdp->sd_freeze_mutex);
return wait_on_bit(&sdp->sd_flags, SDF_FS_FROZEN, TASK_INTERRUPTIBLE);
}
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index a374397f4273..a115c441e2a1 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -91,6 +91,31 @@ int check_journal_clean(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd,
return error;
}
+/**
+ * gfs2_freeze_lock - hold the freeze glock
+ * @sdp: the superblock
+ * @freeze_gh: pointer to the requested holder
+ * @caller_flags: any additional flags needed by the caller
+ */
+int gfs2_freeze_lock(struct gfs2_sbd *sdp, struct gfs2_holder *freeze_gh,
+ int caller_flags)
+{
+ int flags = LM_FLAG_NOEXP | GL_EXACT | caller_flags;
+ int error;
+
+ error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED, flags,
+ freeze_gh);
+ if (error && error != GLR_TRYFAILED)
+ fs_err(sdp, "can't lock the freeze lock: %d\n", error);
+ return error;
+}
+
+void gfs2_freeze_unlock(struct gfs2_holder *freeze_gh)
+{
+ if (gfs2_holder_initialized(freeze_gh))
+ gfs2_glock_dq_uninit(freeze_gh);
+}
+
static void signal_our_withdraw(struct gfs2_sbd *sdp)
{
struct gfs2_glock *gl = sdp->sd_live_gh.gh_gl;
diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
index a4443dd8a94b..69e1a0ae5a4d 100644
--- a/fs/gfs2/util.h
+++ b/fs/gfs2/util.h
@@ -149,6 +149,9 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char *function,
extern int check_journal_clean(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd,
bool verbose);
+extern int gfs2_freeze_lock(struct gfs2_sbd *sdp,
+ struct gfs2_holder *freeze_gh, int caller_flags);
+extern void gfs2_freeze_unlock(struct gfs2_holder *freeze_gh);
#define gfs2_io_error(sdp) \
gfs2_io_error_i((sdp), __func__, __FILE__, __LINE__)
--
2.29.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH 2/2] gfs2: move freeze glock outside the make_fs_rw and _ro functions
2020-12-22 20:43 [Cluster-devel] [GFS2 v2 PATCH 0/2] Circular lock dependency on unmount Bob Peterson
2020-12-22 20:43 ` [Cluster-devel] [GFS2 PATCH 1/2] gfs2: Add common helper for holding and releasing the freeze glock Bob Peterson
@ 2020-12-22 20:43 ` Bob Peterson
2020-12-22 23:58 ` Andreas Gruenbacher
1 sibling, 1 reply; 4+ messages in thread
From: Bob Peterson @ 2020-12-22 20:43 UTC (permalink / raw)
To: cluster-devel.redhat.com
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>
---
fs/gfs2/ops_fstype.c | 31 +++++++++++++++++--------------
fs/gfs2/super.c | 23 -----------------------
fs/gfs2/util.c | 18 ++++++++++++++++--
3 files changed, 33 insertions(+), 39 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c71c7beb9257..cd3fd252c771 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -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;
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index ea312a94ce69..754ea2a137b4 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -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 @@ static void gfs2_dirty_inode(struct inode *inode, int flags)
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)
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index a115c441e2a1..87070e433f96 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -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)
--
2.29.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH 2/2] gfs2: move freeze glock outside the make_fs_rw and _ro functions
2020-12-22 20:43 ` [Cluster-devel] [GFS2 PATCH 2/2] gfs2: move freeze glock outside the make_fs_rw and _ro functions Bob Peterson
@ 2020-12-22 23:58 ` Andreas Gruenbacher
0 siblings, 0 replies; 4+ messages in thread
From: Andreas Gruenbacher @ 2020-12-22 23:58 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Tue, Dec 22, 2020 at 9:43 PM Bob Peterson <rpeterso@redhat.com> wrote:
>
> 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>
> ---
> fs/gfs2/ops_fstype.c | 31 +++++++++++++++++--------------
> fs/gfs2/super.c | 23 -----------------------
> fs/gfs2/util.c | 18 ++++++++++++++++--
> 3 files changed, 33 insertions(+), 39 deletions(-)
>
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index c71c7beb9257..cd3fd252c771 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -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;
>
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index ea312a94ce69..754ea2a137b4 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -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 @@ static void gfs2_dirty_inode(struct inode *inode, int flags)
>
> 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)
> diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
> index a115c441e2a1..87070e433f96 100644
> --- a/fs/gfs2/util.c
> +++ b/fs/gfs2/util.c
> @@ -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))
I'm adding a little cleanup here, but otherwise this patch looks good now:
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -148,15 +148,15 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
gfs2_holder_mark_uninitialized(&freeze_gh);
if (sdp->sd_freeze_gl &&
- !gfs2_glock_is_locked_by_me(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 (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 */
> + 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)
> --
> 2.29.2
>
Thanks,
Andreas
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-22 23:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-22 20:43 [Cluster-devel] [GFS2 v2 PATCH 0/2] Circular lock dependency on unmount Bob Peterson
2020-12-22 20:43 ` [Cluster-devel] [GFS2 PATCH 1/2] gfs2: Add common helper for holding and releasing the freeze glock Bob Peterson
2020-12-22 20:43 ` [Cluster-devel] [GFS2 PATCH 2/2] gfs2: move freeze glock outside the make_fs_rw and _ro functions Bob Peterson
2020-12-22 23:58 ` Andreas Gruenbacher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).