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 <[email protected]>
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);
        }

Reply via email to