| Message ID | 20240524-b4-ksm-scan-optimize-v1-1-053b31bd7ab4@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/ksm: cmp_and_merge_page() optimizations and cleanup | expand |
On 24.05.24 10:56, Chengming Zhou wrote: > In preparation for later changes, refactor out a new function called > try_to_merge_with_zero_page(), which tries to merge with zero page. > > Signed-off-by: Chengming Zhou <chengming.zhou@linux.dev> > --- > mm/ksm.c | 67 +++++++++++++++++++++++++++++++++++----------------------------- > 1 file changed, 37 insertions(+), 30 deletions(-) > > diff --git a/mm/ksm.c b/mm/ksm.c > index 4dc707d175fa..cbd4ba7ea974 100644 > --- a/mm/ksm.c > +++ b/mm/ksm.c > @@ -1531,6 +1531,41 @@ static int try_to_merge_one_page(struct vm_area_struct *vma, > return err; > } > > +/* This function returns 0 if the pages were merged, -EFAULT otherwise. */ No it doesn't. Check the "err = 0" case.
On 2024/5/24 23:12, David Hildenbrand wrote: > On 24.05.24 10:56, Chengming Zhou wrote: >> In preparation for later changes, refactor out a new function called >> try_to_merge_with_zero_page(), which tries to merge with zero page. >> >> Signed-off-by: Chengming Zhou <chengming.zhou@linux.dev> >> --- >> mm/ksm.c | 67 +++++++++++++++++++++++++++++++++++----------------------------- >> 1 file changed, 37 insertions(+), 30 deletions(-) >> >> diff --git a/mm/ksm.c b/mm/ksm.c >> index 4dc707d175fa..cbd4ba7ea974 100644 >> --- a/mm/ksm.c >> +++ b/mm/ksm.c >> @@ -1531,6 +1531,41 @@ static int try_to_merge_one_page(struct vm_area_struct *vma, >> return err; >> } >> +/* This function returns 0 if the pages were merged, -EFAULT otherwise. */ > > No it doesn't. Check the "err = 0" case. > Right, how about this: This function returns 0 if the page were merged or the vma is out of date, which means we don't need to continue, -EFAULT otherwise.
Am 27.05.24 um 06:36 schrieb Chengming Zhou: > On 2024/5/24 23:12, David Hildenbrand wrote: >> On 24.05.24 10:56, Chengming Zhou wrote: >>> In preparation for later changes, refactor out a new function called >>> try_to_merge_with_zero_page(), which tries to merge with zero page. >>> >>> Signed-off-by: Chengming Zhou <chengming.zhou@linux.dev> >>> --- >>> mm/ksm.c | 67 +++++++++++++++++++++++++++++++++++----------------------------- >>> 1 file changed, 37 insertions(+), 30 deletions(-) >>> >>> diff --git a/mm/ksm.c b/mm/ksm.c >>> index 4dc707d175fa..cbd4ba7ea974 100644 >>> --- a/mm/ksm.c >>> +++ b/mm/ksm.c >>> @@ -1531,6 +1531,41 @@ static int try_to_merge_one_page(struct vm_area_struct *vma, >>> return err; >>> } >>> +/* This function returns 0 if the pages were merged, -EFAULT otherwise. */ >> >> No it doesn't. Check the "err = 0" case. >> > > Right, how about this: This function returns 0 if the page were merged or the vma > is out of date, which means we don't need to continue, -EFAULT otherwise. Maybe slightly adjusted: This function returns 0 if the pages were merged or if they are no longer merging candidates (e.g., VMA stale), -EFAULT otherwise.
On 2024/5/27 15:18, David Hildenbrand wrote: > Am 27.05.24 um 06:36 schrieb Chengming Zhou: >> On 2024/5/24 23:12, David Hildenbrand wrote: >>> On 24.05.24 10:56, Chengming Zhou wrote: >>>> In preparation for later changes, refactor out a new function called >>>> try_to_merge_with_zero_page(), which tries to merge with zero page. >>>> >>>> Signed-off-by: Chengming Zhou <chengming.zhou@linux.dev> >>>> --- >>>> mm/ksm.c | 67 +++++++++++++++++++++++++++++++++++----------------------------- >>>> 1 file changed, 37 insertions(+), 30 deletions(-) >>>> >>>> diff --git a/mm/ksm.c b/mm/ksm.c >>>> index 4dc707d175fa..cbd4ba7ea974 100644 >>>> --- a/mm/ksm.c >>>> +++ b/mm/ksm.c >>>> @@ -1531,6 +1531,41 @@ static int try_to_merge_one_page(struct vm_area_struct *vma, >>>> return err; >>>> } >>>> +/* This function returns 0 if the pages were merged, -EFAULT otherwise. */ >>> >>> No it doesn't. Check the "err = 0" case. >>> >> >> Right, how about this: This function returns 0 if the page were merged or the vma >> is out of date, which means we don't need to continue, -EFAULT otherwise. > > Maybe slightly adjusted: > > This function returns 0 if the pages were merged or if they are no longer merging candidates (e.g., VMA stale), -EFAULT otherwise. > Great, will change to this. Thanks!
diff --git a/mm/ksm.c b/mm/ksm.c index 4dc707d175fa..cbd4ba7ea974 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -1531,6 +1531,41 @@ static int try_to_merge_one_page(struct vm_area_struct *vma, return err; } +/* This function returns 0 if the pages were merged, -EFAULT otherwise. */ +static int try_to_merge_with_zero_page(struct ksm_rmap_item *rmap_item, + struct page *page) +{ + struct mm_struct *mm = rmap_item->mm; + int err = -EFAULT; + + /* + * Same checksum as an empty page. We attempt to merge it with the + * appropriate zero page if the user enabled this via sysfs. + */ + if (ksm_use_zero_pages && (rmap_item->oldchecksum == zero_checksum)) { + struct vm_area_struct *vma; + + mmap_read_lock(mm); + vma = find_mergeable_vma(mm, rmap_item->address); + if (vma) { + err = try_to_merge_one_page(vma, page, + ZERO_PAGE(rmap_item->address)); + trace_ksm_merge_one_page( + page_to_pfn(ZERO_PAGE(rmap_item->address)), + rmap_item, mm, err); + } else { + /* + * If the vma is out of date, we do not need to + * continue. + */ + err = 0; + } + mmap_read_unlock(mm); + } + + return err; +} + /* * try_to_merge_with_ksm_page - like try_to_merge_two_pages, * but no new kernel page is allocated: kpage must already be a ksm page. @@ -2305,7 +2340,6 @@ static void stable_tree_append(struct ksm_rmap_item *rmap_item, */ static noinline void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_item) { - struct mm_struct *mm = rmap_item->mm; struct ksm_rmap_item *tree_rmap_item; struct page *tree_page = NULL; struct ksm_stable_node *stable_node; @@ -2374,36 +2408,9 @@ static noinline void cmp_and_merge_page(struct page *page, struct ksm_rmap_item return; } - /* - * Same checksum as an empty page. We attempt to merge it with the - * appropriate zero page if the user enabled this via sysfs. - */ - if (ksm_use_zero_pages && (checksum == zero_checksum)) { - struct vm_area_struct *vma; + if (!try_to_merge_with_zero_page(rmap_item, page)) + return; - mmap_read_lock(mm); - vma = find_mergeable_vma(mm, rmap_item->address); - if (vma) { - err = try_to_merge_one_page(vma, page, - ZERO_PAGE(rmap_item->address)); - trace_ksm_merge_one_page( - page_to_pfn(ZERO_PAGE(rmap_item->address)), - rmap_item, mm, err); - } else { - /* - * If the vma is out of date, we do not need to - * continue. - */ - err = 0; - } - mmap_read_unlock(mm); - /* - * In case of failure, the page was not really empty, so we - * need to continue. Otherwise we're done. - */ - if (!err) - return; - } tree_rmap_item = unstable_tree_search_insert(rmap_item, page, &tree_page); if (tree_rmap_item) {
In preparation for later changes, refactor out a new function called try_to_merge_with_zero_page(), which tries to merge with zero page. Signed-off-by: Chengming Zhou <chengming.zhou@linux.dev> --- mm/ksm.c | 67 +++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 30 deletions(-)