[Cluster-devel] [GFS2 PATCH] GFS2: Reduce number of log flushes

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Wed, 2012-08-08 at 16:10 -0400, Bob Peterson wrote:
> Hi,
> 
> This patch reduces the number of log flushes.
> 
> Regards,
> 
> Bob Peterson
> Red Hat File Systems
> 
> Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
> ---
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 4bdcf37..7d795e7 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -118,7 +118,13 @@ void gfs2_ail_flush(struct gfs2_glock *gl, bool fsync)
>  		return;
>  	__gfs2_ail_flush(gl, fsync);
>  	gfs2_trans_end(sdp);
> -	gfs2_log_flush(sdp, NULL);
> +	/* if fsync is true, we were called from gfs2_fsync, which means
> +	   we just called sync_inode_metadata, which calls sync_inode,
> +	   which calls writeback_single_inode, which calls write_inode,
> +	   which calls write_inode which calls gfs2_log_flush. So the call
> +	   is redundant. */
> +	if (!fsync)
> +		gfs2_log_flush(sdp, NULL);
>  }
What ensures that the revokes are on disk?

>  
>  /**
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 3cbac68..5904ce0 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1542,7 +1542,11 @@ static void gfs2_evict_inode(struct inode *inode)
>  	goto out_unlock;
>  
>  out_truncate:
> -	gfs2_log_flush(sdp, ip->i_gl);
> +	/* write_inode_now calls writeback_single_inode, which calls
> +	   gfs2_write_inode. gfs2_write_inode calls gfs2_log_flush if
> +	   (wbc->sync_mode == WB_SYNC_ALL), which it is since we pass 1.
> +	   therefore, this call to gfs2_log_flush is redundant:
> +	   gfs2_log_flush(sdp, ip->i_gl); */
>  	write_inode_now(inode, 1);
>  	gfs2_ail_flush(ip->i_gl, 0);
>  
> 
This one looks like it is probably correct, however there is no point in
adding a comment to refer to code that has just been removed. Just
remove that call and put the explanation in the patch description, or
maybe include just a short comment so say that there is a log flush
implicit in the write_inode_now() call,

Steve.