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

[xu xin <xu.xin.sc@gmail.com>]

>> From: xu xin <xu.xin16@zte.com.cn>
>>
>> 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.

Thank you.

They are just simply to improve the observability of KSM at process level,
so that users can know if a certain process has enable KSM.

For example, if without these two items, when we look at
/proc/<pid>/ksm_stat and there's no merging pages found, We are not sure
whether it is because KSM was not enabled or because KSM did not
successfully merge any pages.

>
>> 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?

Althrough "mg" in /proc/<pid>/smaps indicate VM_MERGEABLE, it's opaque
and not very obvious for non professionals. 


>>
>>  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.

Yes, okay. Thank you.

>> --- 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()?

Well, the reason why it's not placed inside is to prevent future deadlocks caused by someone using
this function incorrectly under lock already held.

Thanks.