Re: [Cluster-devel] [RHEL7.9.z PATCH 1/2] gfs2: Add common helper for holding and releasing the freeze glock
Hi Bob,
On Thu, Dec 3, 2020 at 4:23 PM Bob Peterson wrote:
> 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
> ---
> fs/gfs2/ops_fstype.c | 6 ++
> fs/gfs2/recovery.c | 6 ++
> fs/gfs2/super.c | 45 +++-
> fs/gfs2/util.c | 35 ++
> fs/gfs2/util.h | 3 +++
> 5 files changed, 57 insertions(+), 38 deletions(-)
>
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index 61fce59cb4d3..4ee56f5e93cb 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 c26c68ebd29d..2208b0e2dc8c 100644
> --- a/fs/gfs2/recovery.c
> +++ b/fs/gfs2/recovery.c
> @@ -470,9 +470,7 @@ void gfs2_recover_func(struct work_struct *work)
>
> /* 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;
>
> @@ -522,7 +520,7 @@ void gfs2_recover_func(struct work_struct *work)
> 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",
The gfs2_glock_dq_uninit(&thaw_gh) at label fail_gunlock_thaw needs to
be turned into gfs2_freeze_unlock(&thaw_gh) as well.
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 2f56acc41c04..801361a05e6f 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)) {
> @@ -614,23 +612,13 @@ int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
> 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)) {
Nooo, please leave the gfs2_glock_is_locked_by_me checking at the few
callers that actually need it. That check doesn't come for free, and
it also makes the code more difficult to understand.
> - if (!log_write_allowed) {
> - error = gfs2_glock_nq_init(sdp->sd_freeze_gl,
> - LM_ST_SHARED, LM_FLAG_TRY |
> -
[Cluster-devel] [RHEL7.9.z PATCH 1/2] gfs2: Add common helper for holding and releasing the freeze glock
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
---
fs/gfs2/ops_fstype.c | 6 ++
fs/gfs2/recovery.c | 6 ++
fs/gfs2/super.c | 45 +++-
fs/gfs2/util.c | 35 ++
fs/gfs2/util.h | 3 +++
5 files changed, 57 insertions(+), 38 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 61fce59cb4d3..4ee56f5e93cb 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 c26c68ebd29d..2208b0e2dc8c 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -470,9 +470,7 @@ void gfs2_recover_func(struct work_struct *work)
/* 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;
@@ -522,7 +520,7 @@ void gfs2_recover_func(struct work_struct *work)
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",
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 2f56acc41c04..801361a05e6f 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)) {
@@ -614,23 +612,13 @@ int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
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)) {
- 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))
-
