Hi,

Comments inline.

----- Original Message -----
> Wake up log waiters in gfs2_log_release when log space has actually become
> available.  This is a much better place for the wakeup than gfs2_logd.
> 
> Check if enough log space is immeditely available before anything else.  If
> there isn't, use io_wait_event to wait instead of open-coding it.
> 
> Signed-off-by: Andreas Gruenbacher <[email protected]>
> ---
>  fs/gfs2/log.c | 54 ++++++++++++++++++++++-----------------------------
>  1 file changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 95ad444bd3dc..7a65823ad1f3 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -420,6 +420,8 @@ void gfs2_log_release(struct gfs2_sbd *sdp, unsigned int
> blks)
>       trace_gfs2_log_blocks(sdp, blks);
>       gfs2_assert_withdraw(sdp, atomic_read(&sdp->sd_log_blks_free) <=
>                                 sdp->sd_jdesc->jd_blocks);
> +     if (atomic_read(&sdp->sd_log_blks_needed))
> +             wake_up(&sdp->sd_log_waitq);
>  }
>  
>  /**
> @@ -444,36 +446,33 @@ void gfs2_log_reserve(struct gfs2_sbd *sdp, unsigned
> int blks)
>  {
>       unsigned reserved_blks = 7 * (4096 / sdp->sd_vfs->s_blocksize);
>       unsigned wanted = blks + reserved_blks;
> -     DEFINE_WAIT(wait);
> -     int did_wait = 0;
>       unsigned int free_blocks;
>  
> -     atomic_add(blks, &sdp->sd_log_blks_needed);
> -retry:
>       free_blocks = atomic_read(&sdp->sd_log_blks_free);
> -     if (unlikely(free_blocks <= wanted)) {
> -             do {
> -                     prepare_to_wait_exclusive(&sdp->sd_log_waitq, &wait,
> -                                     TASK_UNINTERRUPTIBLE);
> +     while (free_blocks >= wanted) {
> +             if (atomic_try_cmpxchg(&sdp->sd_log_blks_free, &free_blocks,
> +                                    free_blocks - blks))
> +                     return;

This would be a good place to have cond_resched() or schedule() or something, 
no?

> +     }
> +
> +     atomic_add(blks, &sdp->sd_log_blks_needed);
> +     for (;;) {
> +             if (current != sdp->sd_logd_process)
>                       wake_up(&sdp->sd_logd_waitq);
> -                     did_wait = 1;
> -                     if (atomic_read(&sdp->sd_log_blks_free) <= wanted)
> -                             io_schedule();
> -                     free_blocks = atomic_read(&sdp->sd_log_blks_free);
> -             } while(free_blocks <= wanted);
> -             finish_wait(&sdp->sd_log_waitq, &wait);
> +             io_wait_event(sdp->sd_log_waitq,
> +                     (free_blocks = atomic_read(&sdp->sd_log_blks_free),
> +                      free_blocks >= wanted));
> +             do {
> +                     if (atomic_try_cmpxchg(&sdp->sd_log_blks_free,
> +                                            &free_blocks,
> +                                            free_blocks - blks))
> +                             goto reserved;

Same here. We need to test these patches with a minimal number of cpus.

> +             } while (free_blocks >= wanted);
>       }
> -     if (atomic_cmpxchg(&sdp->sd_log_blks_free, free_blocks,
> -                             free_blocks - blks) != free_blocks)
> -             goto retry;
> -     atomic_sub(blks, &sdp->sd_log_blks_needed);
> -     trace_gfs2_log_blocks(sdp, -blks);
>  
> -     /*
> -      * If we waited, then so might others, wake them up _after_ we get
> -      * our share of the log.
> -      */
> -     if (unlikely(did_wait))
> +reserved:
> +     trace_gfs2_log_blocks(sdp, -blks);
> +     if (atomic_sub_return(blks, &sdp->sd_log_blks_needed))
>               wake_up(&sdp->sd_log_waitq);
>  }
>  
> @@ -1190,7 +1189,6 @@ int gfs2_logd(void *data)
>       struct gfs2_sbd *sdp = data;
>       unsigned long t = 1;
>       DEFINE_WAIT(wait);
> -     bool did_flush;
>  
>       while (!kthread_should_stop()) {
>  
> @@ -1209,12 +1207,10 @@ int gfs2_logd(void *data)
>                       continue;
>               }
>  
> -             did_flush = false;
>               if (gfs2_jrnl_flush_reqd(sdp) || t == 0) {
>                       gfs2_ail1_empty(sdp, 0);
>                       gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
>                                      GFS2_LFC_LOGD_JFLUSH_REQD);
> -                     did_flush = true;
>               }
>  
>               if (gfs2_ail_flush_reqd(sdp)) {
> @@ -1223,12 +1219,8 @@ int gfs2_logd(void *data)
>                       gfs2_ail1_empty(sdp, 0);
>                       gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
>                                      GFS2_LFC_LOGD_AIL_FLUSH_REQD);
> -                     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;
>  
>               try_to_freeze();
> --
> 2.26.2
> 
> 

Reply via email to