Messages in this thread Patch in this message | | | Date | Thu, 12 May 2022 13:41:11 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH v7] mm/ksm: introduce ksm_force for each process |
| |
On Thu, 12 May 2022 07:03:47 +0000 cgel.zte@gmail.com wrote:
> From: xu xin <xu.xin16@zte.com.cn> > > To use KSM, we have to explicitly call madvise() in application code, > which means installed apps on OS needs to be uninstall and source code > needs to be modified. It is inconvenient. > > In order to change this situation, We add a new proc file ksm_force > under /proc/<pid>/ to support turning on/off KSM scanning of a > process's mm dynamically. > > If ksm_force is set to 1, force all anonymous and 'qualified' VMAs > of this mm to be involved in KSM scanning without explicitly calling > madvise to mark VMA as MADV_MERGEABLE. But It is effective only when > the klob of /sys/kernel/mm/ksm/run is set as 1. > > If ksm_force is set to 0, cancel the feature of ksm_force of this > process and unmerge those merged pages belonging to VMAs which is not > madvised as MADV_MERGEABLE of this process, but leave MADV_MERGEABLE > areas merged.
It certainly seems like a useful feature.
> Signed-off-by: xu xin <xu.xin16@zte.com.cn> > Reviewed-by: Yang Yang <yang.yang29@zte.com.cn> > Reviewed-by: Ran Xiaokai <ran.xiaokai@zte.com.cn> > Reviewed-by: wangyong <wang.yong12@zte.com.cn> > Reviewed-by: Yunkai Zhang <zhang.yunkai@zte.com.cn> > Suggested-by: Matthew Wilcox <willy@infradead.org> > Suggested-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
This patch doesn't have your Signed-off-by:. It should, because you were on the delivery path. This is described in Documentation/process/submitting-patches.rst, "Developer's Certificate of Origin".
I'll queue it for some testing but please do resend with that tag.
> +/* Check if vma is qualified for ksmd scanning */ > +static bool ksm_vma_check(struct vm_area_struct *vma)
I have trouble with "check" names, because the name doesn't convey what is being checked, nor does the name convey whether it's checking for truth or for falsity.
I suggest that "vma_scannable" is a more informative name. It doesn't need the "ksm_" prefix as this is a static file-local function.
See, with the name "vma_scannable", that comment which you added is barely needed.
--- a/mm/ksm.c~mm-ksm-introduce-ksm_force-for-each-process-fix +++ a/mm/ksm.c @@ -335,7 +335,7 @@ static void __init ksm_slab_free(void) } /* Check if vma is qualified for ksmd scanning */ -static bool ksm_vma_check(struct vm_area_struct *vma) +static bool vma_scannable(struct vm_area_struct *vma) { unsigned long vm_flags = vma->vm_flags; @@ -551,7 +551,7 @@ static struct vm_area_struct *find_merge if (ksm_test_exit(mm)) return NULL; vma = vma_lookup(mm, addr); - if (!vma || !ksm_vma_check(vma) || !vma->anon_vma) + if (!vma || !vma_scannable(vma) || !vma->anon_vma) return NULL; return vma; } @@ -2328,7 +2328,7 @@ next_mm: goto no_vmas; for_each_vma(vmi, vma) { - if (!ksm_vma_check(vma)) + if (!vma_scannable(vma)) continue; if (ksm_scan.address < vma->vm_start) ksm_scan.address = vma->vm_start; _
| |