[Cluster-devel] [GFS2 PATCH] GFS2: eliminate gfs2_log_write_bh and simplify

[Steven Whitehouse]

Hi,


On 01/12/17 17:31, Bob Peterson wrote:
> Hi,
>
> This patch makes no functional changes. Its goal is to simplify the
> code and make function gfs2_before_commit more readable. It does
> this by eliminating function gfs2_log_write_bh which was a one-line
> function that simply called gfs2_log_write. It simplifies things
> by introducing a variable bh to reduce the references to bd2->bd_bh.
>
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
While using the new bh variable might improve things, I'm not at all 
convinced that eliminating gfs2_log_write_bh is a good plan, since it is 
a useful helper that is self-documenting,

Steve.

> ---
>   fs/gfs2/lops.c | 37 ++++++++++++-------------------------
>   1 file changed, 12 insertions(+), 25 deletions(-)
>
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index c8ff7b7954f0..438933eb5a25 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -336,21 +336,6 @@ static void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
>   	}
>   }
>   
> -/**
> - * gfs2_log_write_bh - write a buffer's content to the log
> - * @sdp: The super block
> - * @bh: The buffer pointing to the in-place location
> - *
> - * This writes the content of the buffer to the next available location
> - * in the log. The buffer will be unlocked once the i/o to the log has
> - * completed.
> - */
> -
> -static void gfs2_log_write_bh(struct gfs2_sbd *sdp, struct buffer_head *bh)
> -{
> -	gfs2_log_write(sdp, bh->b_page, bh->b_size, bh_offset(bh));
> -}
> -
>   /**
>    * gfs2_log_write_page - write one block stored in a page, into the log
>    * @sdp: The superblock
> @@ -454,25 +439,27 @@ static void gfs2_before_commit(struct gfs2_sbd *sdp, unsigned int limit,
>   
>   		n = 0;
>   		list_for_each_entry_continue(bd2, blist, bd_list) {
> -			get_bh(bd2->bd_bh);
> +			struct buffer_head *bh = bd2->bd_bh;
> +
> +			get_bh(bh);
>   			gfs2_log_unlock(sdp);
> -			lock_buffer(bd2->bd_bh);
> +			lock_buffer(bh);
>   
> -			if (buffer_escaped(bd2->bd_bh)) {
> +			if (buffer_escaped(bh)) {
>   				void *kaddr;
>   				page = mempool_alloc(gfs2_page_pool, GFP_NOIO);
>   				ptr = page_address(page);
> -				kaddr = kmap_atomic(bd2->bd_bh->b_page);
> -				memcpy(ptr, kaddr + bh_offset(bd2->bd_bh),
> -				       bd2->bd_bh->b_size);
> +				kaddr = kmap_atomic(bh->b_page);
> +				memcpy(ptr, kaddr + bh_offset(bh), bh->b_size);
>   				kunmap_atomic(kaddr);
>   				*(__be32 *)ptr = 0;
> -				clear_buffer_escaped(bd2->bd_bh);
> -				unlock_buffer(bd2->bd_bh);
> -				brelse(bd2->bd_bh);
> +				clear_buffer_escaped(bh);
> +				unlock_buffer(bh);
> +				brelse(bh);
>   				gfs2_log_write_page(sdp, page);
>   			} else {
> -				gfs2_log_write_bh(sdp, bd2->bd_bh);
> +				gfs2_log_write(sdp, bh->b_page, bh->b_size,
> +					       bh_offset(bh));
>   			}
>   			gfs2_log_lock(sdp);
>   			if (++n >= num)
>