Re: [PATCH] mm/damon/paddr: Improve readability of damon_pa_stat

[SeongJae Park <sj@kernel.org>]

On Mon, 13 Jan 2025 13:01:56 -0800 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> damon_pa_stat contains an unnecessary goto statement, and the if/else
> can be re-written to be more readable.

Thank you for this nice patch!

Seems this patch conflicts with Usama's patch[1], though.  I believe Andrew
could resolve it on his own, but please let me know if any action is needed.

> 
> This patch is written on top of SJ's patch series [1], which in turn is
> written on top of another one of his series [2].
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

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

> 
> [1] https://lore.kernel.org/all/20241219040327.61902-1-sj@kernel.org/
> [2] https://lore.kernel.org/all/20241213215306.54778-1-sj@kernel.org/

I think 'Signed-off-by:' like tags are the last part of canonical patch
format[2].  Above two links may better to be placed before the tags paragraph
if this patch has a chance to be revised.

[1] https://lore.kernel.org/20250113190738.1156381-1-usamaarif642@gmail.com
[2] https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format

> 
> ---
>  mm/damon/paddr.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index b0c283808ba6..8cdf8320f9a5 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -509,11 +509,8 @@ static unsigned long damon_pa_stat(struct damon_region *r, struct damos *s,
>  		if (!folio)
>  			continue;
>  
> -		if (damos_pa_filter_out(s, folio))
> -			goto put_folio;
> -		else
> +		if (!damos_pa_filter_out(s, folio))
>  			*sz_filter_passed += folio_size(folio);
> -put_folio:
>  		folio_put(folio);
>  	}
>  	return 0;
> 
> base-commit: 2c0573ae5b4c9c0f9d81fa0138ed0014e1baf41b
> -- 
> 2.43.5