* [Cluster-devel] [GFS2 PATCH] GFS2: Combine func meta_prep_new with its only caller
2017-05-22 15:00 ` [Cluster-devel] [GFS2 PATCH] GFS2: Combine func meta_prep_new with its only caller Bob Peterson
@ 2017-05-22 15:11 ` Steven Whitehouse
0 siblings, 0 replies; 2+ messages in thread
From: Steven Whitehouse @ 2017-05-22 15:11 UTC (permalink / raw)
To: cluster-devel.redhat.com
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;
> }
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread