[Cluster-devel] [GFS2 PATCH] gfs2: eliminate ssize parameter from gfs2_struct2blk

[Andrew Price <anprice@redhat.com>]

Hi Bob,

On 13/12/2019 14:46, Bob Peterson wrote:
> Hi,
> 
> Every caller of function gfs2_struct2blk specified sizeof(u64).
> This patch eliminates the unnecessary parameter and replaces the
> size calculation with a new inline function gfs2_ptrs_per_ld to
> compute the maximum number of block pointers we can fit inside a
> log descriptor. This new function will be used later to fix some
> revoke calculations.
> 
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>   fs/gfs2/glops.c |  2 +-
>   fs/gfs2/log.c   | 11 ++++-------
>   fs/gfs2/log.h   | 14 ++++++++++++--
>   fs/gfs2/lops.c  |  2 +-
>   fs/gfs2/trans.c |  3 +--
>   5 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 4ede1f18de85..061d22e1ceb6 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -95,7 +95,7 @@ static void gfs2_ail_empty_gl(struct gfs2_glock *gl)
>   	/* A shortened, inline version of gfs2_trans_begin()
>            * tr->alloced is not set since the transaction structure is
>            * on the stack */
> -	tr.tr_reserved = 1 + gfs2_struct2blk(sdp, tr.tr_revokes, sizeof(u64));
> +	tr.tr_reserved = 1 + gfs2_struct2blk(sdp, tr.tr_revokes);
>   	tr.tr_ip = _RET_IP_;
>   	if (gfs2_log_reserve(sdp, tr.tr_reserved) < 0)
>   		return;
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index eb3f2e7b8085..fe1c35004b10 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -37,7 +37,6 @@ static void gfs2_log_shutdown(struct gfs2_sbd *sdp);
>    * gfs2_struct2blk - compute stuff
>    * @sdp: the filesystem
>    * @nstruct: the number of structures
> - * @ssize: the size of the structures
>    *
>    * Compute the number of log descriptor blocks needed to hold a certain number
>    * of structures of a certain size.
> @@ -45,18 +44,17 @@ static void gfs2_log_shutdown(struct gfs2_sbd *sdp);
>    * Returns: the number of blocks needed (minimum is always 1)
>    */
>   
> -unsigned int gfs2_struct2blk(struct gfs2_sbd *sdp, unsigned int nstruct,
> -			     unsigned int ssize)
> +unsigned int gfs2_struct2blk(struct gfs2_sbd *sdp, unsigned int nstruct)
>   {
>   	unsigned int blks;
>   	unsigned int first, second;
>   
>   	blks = 1;
> -	first = (sdp->sd_sb.sb_bsize - sizeof(struct gfs2_log_descriptor)) / ssize;
> +	first = gfs2_ptrs_per_ld(sdp);
>   
>   	if (nstruct > first) {
>   		second = (sdp->sd_sb.sb_bsize -
> -			  sizeof(struct gfs2_meta_header)) / ssize;
> +			  sizeof(struct gfs2_meta_header)) / sizeof(u64);

Could it be worth storing this value in the sbd (or in jdesc or 
something) so that it only needs to be calculated once?

>   		blks += DIV_ROUND_UP(nstruct - first, second);
>   	}
>   
> @@ -473,8 +471,7 @@ static unsigned int calc_reserved(struct gfs2_sbd *sdp)
>   	}
>   
>   	if (sdp->sd_log_commited_revoke > 0)
> -		reserved += gfs2_struct2blk(sdp, sdp->sd_log_commited_revoke,
> -					  sizeof(u64));
> +		reserved += gfs2_struct2blk(sdp, sdp->sd_log_commited_revoke);
>   	/* One for the overall header */
>   	if (reserved)
>   		reserved++;
> diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
> index 2ff163a8dce1..910fb97f34a1 100644
> --- a/fs/gfs2/log.h
> +++ b/fs/gfs2/log.h
> @@ -60,9 +60,19 @@ static inline void gfs2_ordered_add_inode(struct gfs2_inode *ip)
>   		spin_unlock(&sdp->sd_ordered_lock);
>   	}
>   }
> +
> +/**
> + * gfs2_ptrs_per_ld - compute max number of pointers we can fit in a log desc
> + * @sdp: the filesystem
> + */
> +static inline unsigned int gfs2_ptrs_per_ld(struct gfs2_sbd *sdp)
> +{
> +	return (sdp->sd_sb.sb_bsize - sizeof(struct gfs2_log_descriptor)) /
> +		sizeof(u64);
> +}

This one too?

Andy

> +
>   extern void gfs2_ordered_del_inode(struct gfs2_inode *ip);
> -extern unsigned int gfs2_struct2blk(struct gfs2_sbd *sdp, unsigned int nstruct,
> -			    unsigned int ssize);
> +extern unsigned int gfs2_struct2blk(struct gfs2_sbd *sdp, unsigned int nstruct);
>   
>   extern void gfs2_log_release(struct gfs2_sbd *sdp, unsigned int blks);
>   extern int gfs2_log_reserve(struct gfs2_sbd *sdp, unsigned int blks);
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 12696133618c..3200a6b82487 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -866,7 +866,7 @@ static void revoke_lo_before_commit(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
>   	if (!sdp->sd_log_num_revoke)
>   		return;
>   
> -	length = gfs2_struct2blk(sdp, sdp->sd_log_num_revoke, sizeof(u64));
> +	length = gfs2_struct2blk(sdp, sdp->sd_log_num_revoke);
>   	page = gfs2_get_log_desc(sdp, GFS2_LOG_DESC_REVOKE, length, sdp->sd_log_num_revoke);
>   	offset = sizeof(struct gfs2_log_descriptor);
>   
> diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
> index 9d4227330de4..4d01fe19c125 100644
> --- a/fs/gfs2/trans.c
> +++ b/fs/gfs2/trans.c
> @@ -49,8 +49,7 @@ int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int blocks,
>   	if (blocks)
>   		tr->tr_reserved += 6 + blocks;
>   	if (revokes)
> -		tr->tr_reserved += gfs2_struct2blk(sdp, revokes,
> -						   sizeof(u64));
> +		tr->tr_reserved += gfs2_struct2blk(sdp, revokes);
>   	INIT_LIST_HEAD(&tr->tr_databuf);
>   	INIT_LIST_HEAD(&tr->tr_buf);
>   
>