Re: [PATCH 1/4] hugetlb: Dedup the code to add a new file_region

[Miaohe Lin]

Hi:
On 2021/2/4 5:08, Peter Xu wrote:
> Introduce hugetlb_resv_map_add() helper to add a new file_region rather than
> duplication the similar code twice in add_reservation_in_range().
> 

This cleanup is also in my plan. But I was too sluggish to do this. Many thanks for doing this.
Reviewed-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>

> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
>  mm/hugetlb.c | 51 +++++++++++++++++++++++++++------------------------
>  1 file changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 18f6ee317900..d2859c2aecc9 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -321,6 +321,24 @@ static void coalesce_file_region(struct resv_map *resv, struct file_region *rg)
>  	}
>  }
>  
> +static inline long
> +hugetlb_resv_map_add(struct resv_map *map, struct file_region *rg, long from,
> +		     long to, struct hstate *h, struct hugetlb_cgroup *cg,
> +		     long *regions_needed)
> +{
> +	struct file_region *nrg;
> +
> +	if (!regions_needed) {
> +		nrg = get_file_region_entry_from_cache(map, from, to);
> +		record_hugetlb_cgroup_uncharge_info(cg, h, map, nrg);
> +		list_add(&nrg->link, rg->link.prev);
> +		coalesce_file_region(map, nrg);
> +	} else
> +		*regions_needed += 1;
> +
> +	return to - from;
> +}
> +
>  /*
>   * Must be called with resv->lock held.
>   *
> @@ -336,7 +354,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
>  	long add = 0;
>  	struct list_head *head = &resv->regions;
>  	long last_accounted_offset = f;
> -	struct file_region *rg = NULL, *trg = NULL, *nrg = NULL;
> +	struct file_region *rg = NULL, *trg = NULL;
>  
>  	if (regions_needed)
>  		*regions_needed = 0;
> @@ -365,18 +383,11 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
>  		/* Add an entry for last_accounted_offset -> rg->from, and
>  		 * update last_accounted_offset.
>  		 */
> -		if (rg->from > last_accounted_offset) {
> -			add += rg->from - last_accounted_offset;
> -			if (!regions_needed) {
> -				nrg = get_file_region_entry_from_cache(
> -					resv, last_accounted_offset, rg->from);
> -				record_hugetlb_cgroup_uncharge_info(h_cg, h,
> -								    resv, nrg);
> -				list_add(&nrg->link, rg->link.prev);
> -				coalesce_file_region(resv, nrg);
> -			} else
> -				*regions_needed += 1;
> -		}
> +		if (rg->from > last_accounted_offset)
> +			add += hugetlb_resv_map_add(resv, rg,
> +						    last_accounted_offset,
> +						    rg->from, h, h_cg,
> +						    regions_needed);
>  
>  		last_accounted_offset = rg->to;
>  	}
> @@ -384,17 +395,9 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
>  	/* Handle the case where our range extends beyond
>  	 * last_accounted_offset.
>  	 */
> -	if (last_accounted_offset < t) {
> -		add += t - last_accounted_offset;
> -		if (!regions_needed) {
> -			nrg = get_file_region_entry_from_cache(
> -				resv, last_accounted_offset, t);
> -			record_hugetlb_cgroup_uncharge_info(h_cg, h, resv, nrg);
> -			list_add(&nrg->link, rg->link.prev);
> -			coalesce_file_region(resv, nrg);
> -		} else
> -			*regions_needed += 1;
> -	}
> +	if (last_accounted_offset < t)
> +		add += hugetlb_resv_map_add(resv, rg, last_accounted_offset,
> +					    t, h, h_cg, regions_needed);
>  
>  	VM_BUG_ON(add < 0);
>  	return add;
>