[Cluster-devel] [GFS2 PATCH] GFS2: Combine func meta_prep_new with its only caller

[Cluster-devel] [GFS2 PATCH] GFS2: Combine func meta_prep_new with its only caller

[Bob Peterson]

Hi,

Before this patch function meta_prep_new was only called in one
place, function gfs2_meta_new which is only a couple lines long.
This patch combines them for readability.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 663ffc1..bbd9411 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -158,18 +158,6 @@ struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno, int create)
 	return bh;
 }
 
-static void meta_prep_new(struct buffer_head *bh)
-{
-	struct gfs2_meta_header *mh = (struct gfs2_meta_header *)bh->b_data;
-
-	lock_buffer(bh);
-	clear_buffer_dirty(bh);
-	set_buffer_uptodate(bh);
-	unlock_buffer(bh);
-
-	mh->mh_magic = cpu_to_be32(GFS2_MAGIC);
-}
-
 /**
  * gfs2_meta_new - Get a block
  * @gl: The glock associated with this block
@@ -180,9 +168,15 @@ static void meta_prep_new(struct buffer_head *bh)
 
 struct buffer_head *gfs2_meta_new(struct gfs2_glock *gl, u64 blkno)
 {
-	struct buffer_head *bh;
-	bh = gfs2_getbuf(gl, blkno, CREATE);
-	meta_prep_new(bh);
+	struct buffer_head *bh = gfs2_getbuf(gl, blkno, CREATE);
+	struct gfs2_meta_header *mh = (struct gfs2_meta_header *)bh->b_data;
+
+	lock_buffer(bh);
+	clear_buffer_dirty(bh);
+	set_buffer_uptodate(bh);
+	unlock_buffer(bh);
+
+	mh->mh_magic = cpu_to_be32(GFS2_MAGIC);
 	return bh;
 }
 

[Cluster-devel] [GFS2 PATCH] GFS2: Combine func meta_prep_new with its only caller

[Steven Whitehouse]

Hi,


On 22/05/17 16:00, Bob Peterson wrote:
> Hi,
>
> Before this patch function meta_prep_new was only called in one
> place, function gfs2_meta_new which is only a couple lines long.
> This patch combines them for readability.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
That is a good plan. If you look at the callers of gfs2_meta_new then 
you'll see that there is a common pattern there too (aside from the two 
callers from log.c) and it would be good to merge back some of that into 
this function too, to reduce duplication of code.

It is a bit odd that we unlock the buffer before setting the magic 
number, and also that we do that before adding the buffer to a 
transaction. Worth a quick check there just to see if there is something 
not quite right,

Steve.

> ---
> diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
> index 663ffc1..bbd9411 100644
> --- a/fs/gfs2/meta_io.c
> +++ b/fs/gfs2/meta_io.c
> @@ -158,18 +158,6 @@ struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno, int create)
>   	return bh;
>   }
>   
> -static void meta_prep_new(struct buffer_head *bh)
> -{
> -	struct gfs2_meta_header *mh = (struct gfs2_meta_header *)bh->b_data;
> -
> -	lock_buffer(bh);
> -	clear_buffer_dirty(bh);
> -	set_buffer_uptodate(bh);
> -	unlock_buffer(bh);
> -
> -	mh->mh_magic = cpu_to_be32(GFS2_MAGIC);
> -}
> -
>   /**
>    * gfs2_meta_new - Get a block
>    * @gl: The glock associated with this block
> @@ -180,9 +168,15 @@ static void meta_prep_new(struct buffer_head *bh)
>   
>   struct buffer_head *gfs2_meta_new(struct gfs2_glock *gl, u64 blkno)
>   {
> -	struct buffer_head *bh;
> -	bh = gfs2_getbuf(gl, blkno, CREATE);
> -	meta_prep_new(bh);
> +	struct buffer_head *bh = gfs2_getbuf(gl, blkno, CREATE);
> +	struct gfs2_meta_header *mh = (struct gfs2_meta_header *)bh->b_data;
> +
> +	lock_buffer(bh);
> +	clear_buffer_dirty(bh);
> +	set_buffer_uptodate(bh);
> +	unlock_buffer(bh);
> +
> +	mh->mh_magic = cpu_to_be32(GFS2_MAGIC);
>   	return bh;
>   }
>   
>