[Cluster-devel] [GFS2 PATCH 1/2] GFS2: Simplify gfs2_rgrp_used_recently

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,


On 17/01/18 14:23, Bob Peterson wrote:
> This patch simplifies function gfs2_rgrp_used_recently. Now it uses
> ktime functions to do its calculations. Also, it's called with the
> rgrp rather than the reservation, and the constant HZ rather than
> the hard-coded value 1000.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>   fs/gfs2/rgrp.c | 19 ++++++++-----------
>   1 file changed, 8 insertions(+), 11 deletions(-)
I'm not sure that this does simplify it. In any case the argument is a 
time specified in msec, and HZ is a constant whose value is the number 
of clock ticks in a second. It makes no sense to assign one to the other.

It would be good to get away from the constant of 1 sec that is used 
here, but that required some analysis in order to figure out how to 
create a suitable algorithm. If as a result of testing we can work out a 
more suitable constant to use here, then that is fine as a stop gap 
measure too. HZ though makes no sense at all in this case,

Steve.

> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 6dea72f49316..4c9467bf9ab6 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1900,21 +1900,18 @@ static bool gfs2_rgrp_congested(const struct gfs2_rgrpd *rgd, int loops)
>   }
>   
>   /**
> - * gfs2_rgrp_used_recently
> - * @rs: The block reservation with the rgrp to test
> + * gfs2_rgrp_used_recently - check if a rgrp has been used recently
> + * @rgd: The rgrp to test
>    * @msecs: The time limit in milliseconds
>    *
>    * Returns: True if the rgrp glock has been used within the time limit
>    */
> -static bool gfs2_rgrp_used_recently(const struct gfs2_blkreserv *rs,
> -				    u64 msecs)
> +static inline bool gfs2_rgrp_used_recently(const struct gfs2_rgrpd *rgd,
> +					   u64 msecs)
>   {
> -	u64 tdiff;
> -
> -	tdiff = ktime_to_ns(ktime_sub(ktime_get_real(),
> -                            rs->rs_rbm.rgd->rd_gl->gl_dstamp));
> -
> -	return tdiff > (msecs * 1000 * 1000);
> +	return (ktime_before(ktime_get_real(),
> +			     ktime_add(rgd->rd_gl->gl_dstamp,
> +				       ms_to_ktime(msecs))));
>   }
>   
>   static u32 gfs2_orlov_skip(const struct gfs2_inode *ip)
> @@ -2014,7 +2011,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
>   				    !fast_to_acquire(rs->rs_rbm.rgd))
>   					goto next_rgrp;
>   				if ((loops < 2) &&
> -				    gfs2_rgrp_used_recently(rs, 1000) &&
> +				    gfs2_rgrp_used_recently(rs->rs_rbm.rgd, HZ) &&
>   				    gfs2_rgrp_congested(rs->rs_rbm.rgd, loops))
>   					goto next_rgrp;
>   			}