Re: [PATCH] mm/damon: move the implementation of damon_insert_region to damon.h

[SeongJae Park <sj@kernel.org>]

On Thu, 23 Dec 2021 16:57:03 +0800 Guoqing Jiang <guoqing.jiang@linux.dev> wrote:

> Usually, inline function is declared static since it should sit between
> storage and type. And implement it in a header file if used by multiple
> files.
> 
> And this change also fixes compile issue when backport damon to 5.10.
> 
> mm/damon/vaddr.c: In function ‘damon_va_evenly_split_region’:
> ./include/linux/damon.h:425:13: error: inlining failed in call to ‘always_inline’ ‘damon_insert_region’: function body not available
> 425 | inline void damon_insert_region(struct damon_region *r,
> | ^~~~~~~~~~~~~~~~~~~
> mm/damon/vaddr.c:86:3: note: called from here
> 86 | damon_insert_region(n, r, next, t);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>

Thank you for this patch!

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

> ---
>  include/linux/damon.h | 13 +++++++++++--
>  mm/damon/core.c       | 11 -----------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index b4d4be3cc987..c0b03a8ef0d7 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -422,9 +422,18 @@ struct damon_ctx {
>  #ifdef CONFIG_DAMON
>  
>  struct damon_region *damon_new_region(unsigned long start, unsigned long end);
> -inline void damon_insert_region(struct damon_region *r,
> +
> +/*
> + * Add a region between two other regions
> + */
> +static inline void damon_insert_region(struct damon_region *r,
>  		struct damon_region *prev, struct damon_region *next,
> -		struct damon_target *t);
> +		struct damon_target *t)
> +{
> +	__list_add(&r->list, &prev->list, &next->list);
> +	t->nr_regions++;
> +}
> +
>  void damon_add_region(struct damon_region *r, struct damon_target *t);
>  void damon_destroy_region(struct damon_region *r, struct damon_target *t);
>  
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index e92497895202..01f6347385b4 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -53,17 +53,6 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end)
>  	return region;
>  }
>  
> -/*
> - * Add a region between two other regions
> - */
> -inline void damon_insert_region(struct damon_region *r,
> -		struct damon_region *prev, struct damon_region *next,
> -		struct damon_target *t)
> -{
> -	__list_add(&r->list, &prev->list, &next->list);
> -	t->nr_regions++;
> -}
> -
>  void damon_add_region(struct damon_region *r, struct damon_target *t)
>  {
>  	list_add_tail(&r->list, &t->regions_list);
> -- 
> 2.26.2