Sender: Minchan Kim <minchan.kim@gmail.com>
Date:   Thu, 23 Feb 2023 15:51:01 -0800
From:   Minchan Kim <minchan@kernel.org>
To:     Sergey Senozhatsky <senozhatsky@chromium.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Yosry Ahmed <yosryahmed@google.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCHv2 5/6] zsmalloc: extend compaction statistics
Message-ID: <Y/f75fkcQg6m3cbG@google.com>
References: <20230223030451.543162-1-senozhatsky@chromium.org>
 <20230223030451.543162-6-senozhatsky@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230223030451.543162-6-senozhatsky@chromium.org>
Precedence: bulk

On Thu, Feb 23, 2023 at 12:04:50PM +0900, Sergey Senozhatsky wrote:
> Extend zsmalloc zs_pool_stats with a new member that
> holds the number of objects pool compaction moved
> between pool pages.

I totally understand this new stat would be very useful for your
development but not sure it's really useful for workload tune or
monitoring.

Unless we have strong usecase, I'd like to avoid new stat.

> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  include/linux/zsmalloc.h | 2 ++
>  mm/zsmalloc.c            | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index a48cd0ffe57d..8b3fa5b4a68c 100644
> --- a/include/linux/zsmalloc.h
> +++ b/include/linux/zsmalloc.h
> @@ -36,6 +36,8 @@ enum zs_mapmode {
>  struct zs_pool_stats {
>  	/* How many pages were migrated (freed) */
>  	atomic_long_t pages_compacted;
> +	/* How many objects were migrated during compaction */
> +	atomic_long_t objs_moved;
>  };
>  
>  struct zs_pool;
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index eacf9e32da5c..f7e69df48fb0 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -1815,6 +1815,7 @@ static void migrate_zspage(struct zs_pool *pool, struct size_class *class,
>  		obj_idx++;
>  		record_obj(handle, free_obj);
>  		obj_free(class->size, used_obj, NULL);
> +		atomic_long_inc(&pool->stats.objs_moved);
>  	}