Re: [PATCH V2 2/2] mm: compaction: Limit the value of interface compact_memory

[Vlastimil Babka]

On 3/6/23 07:05, ye.xingchen@xxxxxxxxxx wrote:
> From: Minghao Chi <chi.minghao@xxxxxxxxxx>
> 
> Available only when CONFIG_COMPACTION is set. When 1 is written to
> the file, all zones are compacted such that free memory is available
> in contiguous blocks where possible.
> But echo others-parameter > compact_memory, this function will be
> triggered by writing parameters to the interface.
> 
> Applied this patch,
> sh/$ echo 1.1 > /proc/sys/vm/compact_memory
> sh/$ sh: write error: Invalid argument
> The start and end time of printing triggering compact_memory.
> 
> Link: https://lore.kernel.org/all/ZAJwoXJCzfk1WIBx@xxxxxxxxxxxxxxxxxxxxxx/
> Signed-off-by: Minghao Chi <chi.minghao@xxxxxxxxxx>
> Signed-off-by: Ye Xingchen <ye.xingchen@xxxxxxxxxx>
> ---
>  mm/compaction.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 5a9501e0ae01..2c9ecc4b9d23 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2763,6 +2763,8 @@ int compaction_proactiveness_sysctl_handler(struct ctl_table *table, int write,
>  	return 0;
>  }
> 
> +/* The written value is actually unused, all memory is compacted */
> +int sysctl_compact_memory;
>  /*
>   * This is the entry point for compacting all nodes via
>   * /proc/sys/vm/compact_memory
> @@ -2770,8 +2772,16 @@ int compaction_proactiveness_sysctl_handler(struct ctl_table *table, int write,
>  int sysctl_compaction_handler(struct ctl_table *table, int write,
>  			void *buffer, size_t *length, loff_t *ppos)
>  {
> -	if (write)
> +	int ret;
> +
> +	ret = proc_dointvec_minmax(table, write, buffer, length, ppos);
> +	if (ret)
> +		return ret;
> +	if (write) {
> +		pr_info("compact_nodes start\n");
>  		compact_nodes();
> +		pr_info("compact_nodes end\n");

I'm not sure we want to start spamming the dmesg. This would make sense
if we wanted to deprecate the sysctl and start hunting for remaining
callers to be fixed. Otherwise ftrace can be used to capture e.g. the time.

> +	}
> 
>  	return 0;
>  }