Re: [PATCH linux-next v4] ksm: add ksm involvement information for each process

[Andrew Morton]

On Tue, 3 Dec 2024 19:26:33 +0800 (CST) <xu.xin16@xxxxxxxxxx> wrote:

> From: xu xin <xu.xin16@xxxxxxxxxx>
> 
> In /proc/<pid>/ksm_stat, Add two extra ksm involvement items including
> KSM_mergeable and KSM_merge_any. It helps administrators to
> better know the system's KSM behavior at process level.

It's hard for me to judge the usefulness of this.  Please tell us more:
usage examples, what actions have been taken using this information, etc.

> KSM_mergeable: yes/no
> 	whether any VMAs of the process'mm are currently applicable to KSM.

Could we simply display VM_MERGEABLE in /proc/<pid>/maps?

> KSM_merge_any: yes/no
> 	whether the process'mm is added by prctl() into the candidate list
> 	of KSM or not, and fully enabled at process level.
> 
> ...
>
>  fs/proc/base.c      | 11 +++++++++++
>  include/linux/ksm.h |  1 +
>  mm/ksm.c            | 19 +++++++++++++++++++

Documentation/admin-guide/mm/ksm.rst will require an update please.

>
> ...
>
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -3269,6 +3269,7 @@ static int proc_pid_ksm_stat(struct seq_file *m, struct pid_namespace *ns,
>  				struct pid *pid, struct task_struct *task)
>  {
>  	struct mm_struct *mm;
> +	int ret = 0;
> 
>  	mm = get_task_mm(task);
>  	if (mm) {
> @@ -3276,6 +3277,16 @@ static int proc_pid_ksm_stat(struct seq_file *m, struct pid_namespace *ns,
>  		seq_printf(m, "ksm_zero_pages %ld\n", mm_ksm_zero_pages(mm));
>  		seq_printf(m, "ksm_merging_pages %lu\n", mm->ksm_merging_pages);
>  		seq_printf(m, "ksm_process_profit %ld\n", ksm_process_profit(mm));
> +		seq_printf(m, "ksm_merge_any: %s\n",
> +				test_bit(MMF_VM_MERGE_ANY, &mm->flags) ? "yes" : "no");
> +		ret = mmap_read_lock_killable(mm);

Could do the locking in ksm_process_mergeable()?

> +		if (ret) {
> +			mmput(mm);
> +			return ret;
> +		}
> +		seq_printf(m, "ksm_mergeable: %s\n",
> +				ksm_process_mergeable(mm) ? "yes" : "no");

Calling seq_printf() after the mmap_read_unlock() would be a little
more scalable.

> +		mmap_read_unlock(mm);
>  		mmput(mm);
>  	}
>
> ...
>
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -3263,6 +3263,25 @@ static void wait_while_offlining(void)
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
> 
>  #ifdef CONFIG_PROC_FS
> +/*
> + * The process is mergeable only if any VMA (and which) is currently
> + * applicable to KSM.

That sentence needs revisiting, please.

> + * The mmap lock must be held in read mode.
> + */
> +bool ksm_process_mergeable(struct mm_struct *mm)
> +{
> +	struct vm_area_struct *vma;
> +
> +	mmap_assert_locked(mm);
> +	VMA_ITERATOR(vmi, mm, 0);
> +	for_each_vma(vmi, vma)
> +		if (vma->vm_flags & VM_MERGEABLE)
> +			return true;
> +
> +	return false;
> +}