[Cluster-devel] [GFS2 PATCH] GFS2: Rework gfs2_logd daemon
Steven Whitehouse
swhiteho at redhat.com
Mon Jul 3 09:32:09 UTC 2017
Hi,
On 30/06/17 17:45, Bob Peterson wrote:
> Hi,
>
> This patch reorganizes some of the hokey logic in gfs2_logd.
> It also tries to only fetch the logd_secs tunable only once per
> second to avoid too many spinlock conflicts.
>
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
Again, please explain what the benefit is. The tunable should only every
be accessed by gfs2_logd and by a userland process that is updating the
tunable. If there is contention here, something is very wrong. The patch
appears to remove the check for whether a journal or ail flush is
required before gfs2_logd sleeps, so this appears that it would likely
make performance worse,
Steve.
> ---
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 32aa1f0..3f18db4 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -915,15 +915,12 @@ int gfs2_logd(void *data)
> struct gfs2_sbd *sdp = data;
> unsigned long t = 1;
> DEFINE_WAIT(wait);
> - bool did_flush;
> + unsigned long tune_time = 0;
>
> while (!kthread_should_stop()) {
> -
> - did_flush = false;
> if (gfs2_jrnl_flush_reqd(sdp) || t == 0) {
> gfs2_ail1_empty(sdp);
> gfs2_log_flush(sdp, NULL, NORMAL_FLUSH);
> - did_flush = true;
> }
>
> if (gfs2_ail_flush_reqd(sdp)) {
> @@ -931,26 +928,22 @@ int gfs2_logd(void *data)
> gfs2_ail1_wait(sdp);
> gfs2_ail1_empty(sdp);
> gfs2_log_flush(sdp, NULL, NORMAL_FLUSH);
> - did_flush = true;
> }
>
> - if (!gfs2_ail_flush_reqd(sdp) || did_flush)
> - wake_up(&sdp->sd_log_waitq);
> -
> - t = gfs2_tune_get(sdp, gt_logd_secs) * HZ;
> + wake_up(&sdp->sd_log_waitq);
>
> try_to_freeze();
>
> - do {
> - prepare_to_wait(&sdp->sd_logd_waitq, &wait,
> - TASK_INTERRUPTIBLE);
> - if (!gfs2_ail_flush_reqd(sdp) &&
> - !gfs2_jrnl_flush_reqd(sdp) &&
> - !kthread_should_stop())
> - t = schedule_timeout(t);
> - } while(t && !gfs2_ail_flush_reqd(sdp) &&
> - !gfs2_jrnl_flush_reqd(sdp) &&
> - !kthread_should_stop());
> + if (kthread_should_stop())
> + break;
> +
> + if (time_after(jiffies, tune_time + HZ)) {
> + t = gfs2_tune_get(sdp, gt_logd_secs) * HZ;
> + tune_time = jiffies;
> + }
> + prepare_to_wait(&sdp->sd_logd_waitq, &wait,
> + TASK_INTERRUPTIBLE);
> + t = schedule_timeout(t);
> finish_wait(&sdp->sd_logd_waitq, &wait);
> }
>
More information about the Cluster-devel
mailing list