* [PATCH v2 0/4] KSM: introduce only_merge_zero_pages
@ 2026-11-10 7:43 Xu Xin (vivo)
2026-11-10 7:44 ` [PATCH 1/4] ksm: add only_merge_zero_pages to merge only empty anonymous pages Xu Xin (vivo)
2026-11-10 10:08 ` [PATCH v2 0/4] KSM: introduce only_merge_zero_pages Lorenzo Stoakes (ARM)
0 siblings, 2 replies; 10+ messages in thread
From: Xu Xin (vivo) @ 2026-11-10 7:43 UTC (permalink / raw)
To: David Hildenbrand, Xu Xin, Chengming Zhou, Andrew Morton,
Lorenzo Stoakes, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Jonathan Corbet, Jinjiang Tu, Qiliang Yuan
Cc: linux-mm, linux-doc, linux-kernel, linux-kselftest, Huan Yang, Wang Xiao
Hi all,
This series adds a new KSM sysfs attribute, only_merge_zero_pages, which
makes ksmd merge only empty (zeroed) anonymous pages with the kernel
zero page and skip the stable/unstable trees for pages that carry data.
Patch 1 adds the attribute and the early return in cmp_and_merge_page().
Patch 2 (co-developed with Jinjiang Tu) goes one step further and skips
non-zero pages already in the scan path, before an rmap_item is
allocated for them, which is what makes the mode practical on
large-memory workloads with mostly non-mergeable anonymous memory.
Patch 3 consolidates the ksm_stat readers in vm_util.c into a single
ksm_get_self_stat() helper (pure refactor). Patch 4 adds a functional
selftest.
Changelog
=========
v1 -> v2
- Patch 1: Take ksm_thread_mutex around the check-and-set in both
use_zero_pages_store() and only_merge_zero_pages_store(), so
that a concurrent write to the two attributes cannot leave
use_zero_pages=0 and only_merge_zero_pages=1, a state in which
ksmd would scan and checksum pages but never merge any.
- New patch 2, 3 and 4 are added.
Motivation
==========
On an Android device, I measured the KSM savings potential on 15.7 GiB
RAM, ~1100 processes, a typical usage session with several video apps
running) using a userspace survey tool that walks /proc/<pid>/maps +
/proc/<pid>/pagemap, deduplicates anonymous pages by their physical
frame number (PFN), and hashes page contents to find identical pages.
Key results (two samples taken 10 minutes apart):
Category Resident Savings (upper bound)
------------------- --------- ---------------------
anon, non-zero pages 2.9G 67-74 MiB (2.3-2.4%)
anon, zero pages - 281-424 MiB (9.5-13.9%)
In other scenario with a MySQL instance from huawei, there are around
15 GB consumed by hashtables, which contain many zero-filled pages that
are eligible for merging to reduce memory footprint [1].
[1] https://lore.kernel.org/all/2a642af6-b97a-4f24-9889-1844812de4cc@huawei.com/
Xu Xin (vivo) (4):
ksm: add only_merge_zero_pages to merge only empty anonymous pages
ksm: skip rmap_item allocation for non-zero pages under
only_merge_zero_pages
selftests/ksm: consolidate ksm_stat readers into ksm_get_self_stat()
selftests/ksm: add test for only_merge_zero_pages
Documentation/admin-guide/mm/ksm.rst | 40 ++++-
mm/ksm.c | 149 ++++++++++++++++++
.../selftests/mm/ksm_functional_tests.c | 135 ++++++++++++++--
tools/testing/selftests/mm/vm_util.c | 40 ++---
tools/testing/selftests/mm/vm_util.h | 3 +-
5 files changed, 327 insertions(+), 40 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/4] ksm: add only_merge_zero_pages to merge only empty anonymous pages 2026-11-10 7:43 [PATCH v2 0/4] KSM: introduce only_merge_zero_pages Xu Xin (vivo) @ 2026-11-10 7:44 ` Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages Xu Xin (vivo) ` (2 more replies) 2026-11-10 10:08 ` [PATCH v2 0/4] KSM: introduce only_merge_zero_pages Lorenzo Stoakes (ARM) 1 sibling, 3 replies; 10+ messages in thread From: Xu Xin (vivo) @ 2026-11-10 7:44 UTC (permalink / raw) To: xu.xin Cc: akpm, chengming.zhou, corbet, david, link, linux-doc, linux-kernel, linux-kselftest, linux-mm, ljs, odys.yuan, rppt, surenb, tujinjiang, vbabka, xiao.wang Add a new sysfs attribute, /sys/kernel/mm/ksm/only_merge_zero_pages, which, when enabled, makes ksmd merge only empty (zeroed) anonymous pages with the kernel zero page and skip the stable and unstable trees entirely for pages that carry data. The attribute requires use_zero_pages to be 1, and the two are kept consistent by symmetric constraints: it cannot be enabled while use_zero_pages is 0 (-EINVAL), and use_zero_pages cannot be turned off while it is active (-EBUSY). Both store handlers take ksm_thread_mutex around their check-and-set so that a concurrent write to the two attributes cannot leave use_zero_pages=0 and only_merge_zero_pages=1, a state in which ksmd would scan and checksum pages but never merge any. The new attribute can be changed at any time and is only effective for pages merged after the change. Benefits in three aspects ========================= - Memory saving: On memory-constrained systems such as Android and embedded devices, zero-filled anonymous pages are the largest source of duplicate anonymous memory. They are produced by userspace allocators: the zeroing of freshly allocated Java objects and buffers in the ART heap (new byte[]/int[], Bitmap buffers), calloc/memset in native code, and preallocated media buffers. Merging them with the kernel zero page frees a significant amount of physical memory, which matters as DRAM costs keep rising. With this enabled, we measured 100MB–300MB saving tested with several popular applications on aarch64 (Android). - CPU overhead reduction: Merging only zero pages also reduces the overhead of ksmd. A page that is not merged with the zero page carries data and can never be merged with the zero page, so there is no point in searching the stable tree or inserting it into the unstable tree for it. Skipping those tree operations for data-bearing pages reduces both the CPU time and the rhashtable memory footprint of ksmd, which makes zero page merging practical on low-resource devices where full KSM scanning would be too expensive. - Security: Finally, this shrinks the KSM side-channel attack surface: only zero pages are ever merged, so the sharing status of data-bearing pages is no longer observable through KSM timing side channels. Note ===== - Suitable for Android, embedded and low-memory devices, where zero-page deduplication is wanted but full KSM scanning is too costly, and reduced KSM side-channel exposure is desired. Many handset vendors may prefer this knob, which can be used as a default fallback policy. - Not suitable for server and virtualization workloads containing many non-zero duplicate pages, where vanilla KSM provides better memory savings. Signed-off-by: Xu Xin (vivo) <xu.xin@linux.dev> Reviewed-by: Huan Yang <link@vivo.com> Cc: Wang Xiao <xiao.wang@vivo.com> --- Documentation/admin-guide/mm/ksm.rst | 40 ++++++++++++++++- mm/ksm.c | 67 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst index c329ca747b8c..7c2cbac4b020 100644 --- a/Documentation/admin-guide/mm/ksm.rst +++ b/Documentation/admin-guide/mm/ksm.rst @@ -128,10 +128,48 @@ use_zero_pages KSM for some workloads, for example if the checksums of pages candidate for merging match the checksum of an empty page. This setting can be changed at any time, it is only - effective for pages merged after the change. + effective for pages merged after the change. Turning this off + while ``only_merge_zero_pages`` is enabled fails with EBUSY; + clear ``only_merge_zero_pages`` first. Default: 0 (normal KSM behaviour as in earlier releases) +only_merge_zero_pages + specifies whether ksmd should only merge empty (zeroed) + anonymous pages, with the kernel zero page, and skip the stable + and unstable trees entirely for pages that carry data. This + requires ``use_zero_pages`` to be 1; it cannot be enabled while + ``use_zero_pages`` is 0, and ``use_zero_pages`` cannot be turned + off while it is enabled (the write fails with EBUSY). + + This is useful on memory-constrained systems (e.g. Android and + embedded devices) where the main source of duplicate anonymous + memory is zero-filled pages produced by userspace allocators + (for example the zeroing of freshly allocated Java objects and + buffers in the ART heap, ``calloc``/``memset`` in native code, + and pre-allocated media buffers). Merging only those pages + avoids the per-page cost of searching and inserting into the + stable and unstable trees for data-bearing pages, which reduces + the CPU and memory overhead of ksmd. It also shrinks the KSM + side-channel attack surface, since only zero pages are ever + merged and the sharing state of data-bearing pages is no longer + observable through KSM. + + - Suitable for: Android, embedded and low-memory devices, where + zero-page deduplication is wanted but full KSM scanning is + too costly, and reduced KSM side-channel exposure is desired. + Many handset vendors may prefer this knob, which can be used + as a default fallback policy. + + - Not suitable for: Server and virtualization workloads containing + many non-zero duplicate pages, where vanilla KSM provides better + memory savings. + + This setting can be changed at any time, it is only effective + for pages merged after the change. + + Default: 0 (normal KSM behaviour) + max_page_sharing Maximum sharing allowed for each KSM page. This enforces a deduplication limit to avoid high latency for virtual memory diff --git a/mm/ksm.c b/mm/ksm.c index 5e24b5ca8dc8..49f3728a3d03 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -295,6 +295,13 @@ static unsigned int zero_checksum __read_mostly; /* Whether to merge empty (zeroed) pages with actual zero pages */ static bool ksm_use_zero_pages __read_mostly; +/* + * When set, ksmd only merges empty (zeroed) anonymous pages with the kernel + * zero page and skips the stable/unstable tree entirely for pages that carry + * data. This requires ksm_use_zero_pages to be enabled. + */ +static bool ksm_only_merge_zero_pages __read_mostly; + /* Skip pages that couldn't be de-duplicated previously */ /* Default to true at least temporarily, for testing */ static bool ksm_smart_scan = true; @@ -2323,6 +2330,14 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite if (!try_to_merge_with_zero_page(rmap_item, page)) return; + + /* + * A page that reached here was not merged with the zero page this + * round. Under only_merge_zero_pages we do not insert it into the + * stable/unstable trees. + */ + if (ksm_only_merge_zero_pages) + return; } /* Start by searching for the folio in the stable tree */ @@ -3697,12 +3712,63 @@ static ssize_t use_zero_pages_store(struct kobject *kobj, if (err) return -EINVAL; + /* + * only_merge_zero_pages depends on use_zero_pages: refuse to turn + * the latter off while the former is active, so the two cannot + * silently diverge. Take ksm_thread_mutex so that this check-and-set + * is serialized against only_merge_zero_pages_store(), which reads + * ksm_use_zero_pages under the same lock. + */ + mutex_lock(&ksm_thread_mutex); + if (!value && ksm_only_merge_zero_pages) { + mutex_unlock(&ksm_thread_mutex); + return -EBUSY; + } ksm_use_zero_pages = value; + mutex_unlock(&ksm_thread_mutex); return count; } KSM_ATTR(use_zero_pages); +static ssize_t only_merge_zero_pages_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%u\n", ksm_only_merge_zero_pages); +} +static ssize_t only_merge_zero_pages_store(struct kobject *kobj, + struct kobj_attribute *attr, + const char *buf, size_t count) +{ + int err; + bool value; + + err = kstrtobool(buf, &value); + if (err) + return -EINVAL; + + /* + * Take ksm_thread_mutex so that this check-and-set is serialized + * against use_zero_pages_store(), which reads ksm_only_merge_zero_pages + * under the same lock. Without it, a concurrent write to the two + * attributes could leave use_zero_pages=0 and only_merge_zero_pages=1, + * a state in which ksmd would scan and checksum pages but never merge + * any. + */ + mutex_lock(&ksm_thread_mutex); + if (value && !ksm_use_zero_pages) { + mutex_unlock(&ksm_thread_mutex); + pr_warn_once("ksm: only_merge_zero_pages can only be enabled " + "when use_zero_pages is 1\n"); + return -EINVAL; + } + ksm_only_merge_zero_pages = value; + mutex_unlock(&ksm_thread_mutex); + + return count; +} +KSM_ATTR(only_merge_zero_pages); + static ssize_t max_page_sharing_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { @@ -4028,6 +4094,7 @@ static struct attribute *ksm_attrs[] = { &stable_node_dups_attr.attr, &stable_node_chains_prune_millisecs_attr.attr, &use_zero_pages_attr.attr, + &only_merge_zero_pages_attr.attr, &general_profit_attr.attr, &smart_scan_attr.attr, &advisor_mode_attr.attr, -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages 2026-11-10 7:44 ` [PATCH 1/4] ksm: add only_merge_zero_pages to merge only empty anonymous pages Xu Xin (vivo) @ 2026-11-10 7:44 ` Xu Xin (vivo) 2026-11-10 10:05 ` Lorenzo Stoakes (ARM) 2026-11-10 7:44 ` [PATCH 3/4] selftests/ksm: consolidate ksm_stat readers into ksm_get_self_stat() Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 4/4] selftests/ksm: add test for only_merge_zero_pages Xu Xin (vivo) 2 siblings, 1 reply; 10+ messages in thread From: Xu Xin (vivo) @ 2026-11-10 7:44 UTC (permalink / raw) To: xu.xin Cc: akpm, chengming.zhou, corbet, david, link, linux-doc, linux-kernel, linux-kselftest, linux-mm, ljs, odys.yuan, rppt, surenb, tujinjiang, vbabka, xiao.wang When only_merge_zero_pages is enabled, ksmd only ever merges empty (zeroed) anonymous pages with the kernel zero page. A page that carries data can never be merged, yet the current code still allocates an rmap_item for it in scan_get_next_rmap_item() before cmp_and_merge_page() decides to skip it. On workloads with a large amount of non-mergeable anonymous memory this is a real cost: every scanned non-zero page pays for an rmap_item allocation, and the rmap_items accumulate in the per-mm pending list. For example, a MySQL instance with hundreds of gigabytes of anonymous memory (mostly buffer pool pages that cannot be merged) ends up with ksmd creating rmap_item metadata for each candidate page; the metadata overhead can wipe out any gain from the zero-page merging and even turn the net benefit negative. Add should_skip_non_zero_page(), called in scan_get_next_rmap_item() before get_next_rmap_item(), which, when only_merge_zero_pages is enabled, checks whether the candidate page is all zeros. A non-zero page is skipped entirely: no rmap_item is allocated for it, and any stale pending rmap_items below it are freed (as get_next_rmap_item() would). The page is reported back to ksm_do_scan() as ERR_PTR(-EAGAIN), which counts it as scanned and continues, so the pages_to_scan budget is preserved. The all-zero check (pages_identical() against the kernel zero page) reads the whole page. For a non-zero page this is a net win: the extra page read is cheaper than the rmap_item allocation and the stable/unstable tree operations it would otherwise trigger. A page that is zero now but is written before cmp_and_merge_page() runs is still handled correctly, because the checksum-based decision in cmp_and_merge_page() (added by the previous patch) is the authoritative one. Signed-off-by: Xu Xin (vivo) <xu.xin@linux.dev> Co-developed-by: Jinjiang Tu <tujinjiang@huawei.com> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> --- mm/ksm.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/mm/ksm.c b/mm/ksm.c index 49f3728a3d03..e641c37ee096 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -2535,6 +2535,69 @@ static bool should_skip_rmap_item(struct folio *folio, return true; } +/* + * Determines whether a candidate page should be skipped entirely when + * only_merge_zero_pages is enabled, before an rmap_item is allocated for + * it. + * + * A page that is not all zeros carries data and can never be merged with + * the kernel zero page, so there is no point in allocating an rmap_item + * for it. Skipping it here avoids the rmap_item metadata cost that full + * KSM would otherwise pay for every non-mergeable page. + * + * @rmap_list: head of the per-mm pending rmap_item list + * @addr: address of the candidate page + * @folio: folio containing the candidate page + * @page: the candidate page + * + * Returns true if the page should be skipped (and any stale pending + * rmap_items below @addr are freed), false if it should be processed. + */ +static bool should_skip_non_zero_page(struct ksm_rmap_item **rmap_list, + unsigned long addr, struct folio *folio, + struct page *page) +{ + struct ksm_rmap_item *rmap_item; + + if (!ksm_only_merge_zero_pages) + return false; + + /* + * Pages already merged by KSM are handled by cmp_and_merge_page(); + * do not skip them here. + */ + if (folio_test_ksm(folio)) + return false; + + /* + * An all-zero page is exactly what we want to merge with the kernel + * zero page; process it. + */ + if (pages_identical(page, ZERO_PAGE(0))) + return false; + + /* + * The page carries data: free any pending rmap_items that are below + * it (they are stale, as get_next_rmap_item() would), and skip it so + * no rmap_item is allocated for it. + * + * Compare the masked address, unlike get_next_rmap_item() which + * compares the raw rmap_item->address (which may carry the + * UNSTABLE/STABLE_FLAG bits). That is intentional: a pending + * rmap_item at the same address as a non-zero page is stale and must + * be freed here, not returned for processing. + */ + while (*rmap_list) { + rmap_item = *rmap_list; + if ((rmap_item->address & PAGE_MASK) > addr) + break; + *rmap_list = rmap_item->rmap_list; + remove_rmap_item_from_tree(rmap_item); + free_rmap_item(rmap_item); + } + return true; +} + struct ksm_next_page_arg { struct folio *folio; struct page *page; @@ -2737,6 +2800,16 @@ static struct ksm_rmap_item *scan_get_next_rmap_item(struct page **page) if (tmp_page) { flush_anon_page(vma, tmp_page, ksm_scan.address); flush_dcache_page(tmp_page); + + if (should_skip_non_zero_page(ksm_scan.rmap_list, + ksm_scan.address, folio, + tmp_page)) { + folio_put(folio); + ksm_scan.address += PAGE_SIZE; + mmap_read_unlock(mm); + return ERR_PTR(-EAGAIN); + } + rmap_item = get_next_rmap_item(mm_slot, ksm_scan.rmap_list, ksm_scan.address); if (rmap_item) { @@ -2839,6 +2912,15 @@ static void ksm_do_scan(unsigned int scan_npages) rmap_item = scan_get_next_rmap_item(&page); if (!rmap_item) return; + if (rmap_item == ERR_PTR(-EAGAIN)) { + /* + * A non-zero page was skipped under + * only_merge_zero_pages: it was examined but not + * merged, so count it as scanned and keep going. + */ + ksm_pages_scanned++; + continue; + } cmp_and_merge_page(page, rmap_item); put_page(page); ksm_pages_scanned++; -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages 2026-11-10 7:44 ` [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages Xu Xin (vivo) @ 2026-11-10 10:05 ` Lorenzo Stoakes (ARM) 2026-11-10 11:22 ` Xu Xin (vivo) 0 siblings, 1 reply; 10+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-11-10 10:05 UTC (permalink / raw) To: Xu Xin (vivo) Cc: akpm, chengming.zhou, corbet, david, link, linux-doc, linux-kernel, linux-kselftest, linux-mm, odys.yuan, rppt, surenb, tujinjiang, vbabka, xiao.wang On Tue, Nov 10, 2026 at 03:44:49PM +0800, Xu Xin (vivo) wrote: > When only_merge_zero_pages is enabled, ksmd only ever merges empty > (zeroed) anonymous pages with the kernel zero page. A page that carries > data can never be merged, yet the current code still allocates an > rmap_item for it in scan_get_next_rmap_item() before cmp_and_merge_page() > decides to skip it. > > On workloads with a large amount of non-mergeable anonymous memory this > is a real cost: every scanned non-zero page pays for an rmap_item > allocation, and the rmap_items accumulate in the per-mm pending list. > For example, a MySQL instance with hundreds of gigabytes of anonymous > memory (mostly buffer pool pages that cannot be merged) ends up with > ksmd creating rmap_item metadata for each candidate page; the metadata > overhead can wipe out any gain from the zero-page merging and even turn > the net benefit negative. > > Add should_skip_non_zero_page(), called in scan_get_next_rmap_item() before > get_next_rmap_item(), which, when only_merge_zero_pages is enabled, > checks whether the candidate page is all zeros. A non-zero page is > skipped entirely: no rmap_item is allocated for it, and any stale > pending rmap_items below it are freed (as get_next_rmap_item() would). > The page is reported back to ksm_do_scan() as ERR_PTR(-EAGAIN), which > counts it as scanned and continues, so the pages_to_scan budget is > preserved. > > The all-zero check (pages_identical() against the kernel zero page) reads > the whole page. For a non-zero page this is a net win: the extra page read > is cheaper than the rmap_item allocation and the stable/unstable tree > operations it would otherwise trigger. > > A page that is zero now but is written before cmp_and_merge_page() runs > is still handled correctly, because the checksum-based decision in > cmp_and_merge_page() (added by the previous patch) is the authoritative > one. > Please re-write this in your own words. It's rambling on and never gets to the point of _what this patch does and why_. I'm not interested in reading a novel to understand the code :) It seems very LLM generated, LLMs are _terrible_ at writing commit messages and commits, please re-write all of that. > Signed-off-by: Xu Xin (vivo) <xu.xin@linux.dev> > Co-developed-by: Jinjiang Tu <tujinjiang@huawei.com> > Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> Why are this and the other patches in-reply-to 1/4? That's not how you send a series in mm. > --- > mm/ksm.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 82 insertions(+) > > diff --git a/mm/ksm.c b/mm/ksm.c > index 49f3728a3d03..e641c37ee096 100644 > --- a/mm/ksm.c > +++ b/mm/ksm.c > @@ -2535,6 +2535,69 @@ static bool should_skip_rmap_item(struct folio *folio, > return true; > } > > +/* > + * Determines whether a candidate page should be skipped entirely when > + * only_merge_zero_pages is enabled, before an rmap_item is allocated for > + * it. > + * > + * A page that is not all zeros carries data and can never be merged with > + * the kernel zero page, so there is no point in allocating an rmap_item > + * for it. Skipping it here avoids the rmap_item metadata cost that full > + * KSM would otherwise pay for every non-mergeable page. > + * > + * @rmap_list: head of the per-mm pending rmap_item list > + * @addr: address of the candidate page > + * @folio: folio containing the candidate page > + * @page: the candidate page > + * > + * Returns true if the page should be skipped (and any stale pending > + * rmap_items below @addr are freed), false if it should be processed. > + */ This is a really weird mix of kdoc style + not kdoc style while not being a kdoc, for a static function? > +static bool should_skip_non_zero_page(struct ksm_rmap_item **rmap_list, > + unsigned long addr, struct folio *folio, > + struct page *page) > +{ > + struct ksm_rmap_item *rmap_item; > + > + if (!ksm_only_merge_zero_pages) > + return false; > + > + /* > + * Pages already merged by KSM are handled by cmp_and_merge_page(); > + * do not skip them here. > + */ > + if (folio_test_ksm(folio)) > + return false; > + > + /* > + * An all-zero page is exactly what we want to merge with the kernel > + * zero page; process it. > + */ > + if (pages_identical(page, ZERO_PAGE(0))) > + return false; > + > + /* > + * The page carries data: free any pending rmap_items that are below > + * it (they are stale, as get_next_rmap_item() would), and skip it so > + * no rmap_item is allocated for it. > + * > + * Compare the masked address, unlike get_next_rmap_item() which > + * compares the raw rmap_item->address (which may carry the > + * UNSTABLE/STABLE_FLAG bits). That is intentional: a pending > + * rmap_item at the same address as a non-zero page is stale and must > + * be freed here, not returned for processing. > + */ We don't need a prose version of the code, and it reads like AI slop, we don't want AI slop comments, thanks. > + while (*rmap_list) { > + rmap_item = *rmap_list; Why is rmap_item function-scoped when it could be block-scoped here... > + if ((rmap_item->address & PAGE_MASK) > addr) > + break; > + *rmap_list = rmap_item->rmap_list; > + remove_rmap_item_from_tree(rmap_item); > + free_rmap_item(rmap_item); > + } This code is very ugly. Newline... > + return true; > +} > + > struct ksm_next_page_arg { > struct folio *folio; > struct page *page; > @@ -2737,6 +2800,16 @@ static struct ksm_rmap_item *scan_get_next_rmap_item(struct page **page) > if (tmp_page) { > flush_anon_page(vma, tmp_page, ksm_scan.address); > flush_dcache_page(tmp_page); > + > + if (should_skip_non_zero_page(ksm_scan.rmap_list, > + ksm_scan.address, folio, > + tmp_page)) { > + folio_put(folio); > + ksm_scan.address += PAGE_SIZE; > + mmap_read_unlock(mm); > + return ERR_PTR(-EAGAIN); > + } > + I don't love just shoving more logic into an already massive function. No to this. At some point technical debt has to be paid, clean up this code first please. > rmap_item = get_next_rmap_item(mm_slot, > ksm_scan.rmap_list, ksm_scan.address); > if (rmap_item) { > @@ -2839,6 +2912,15 @@ static void ksm_do_scan(unsigned int scan_npages) > rmap_item = scan_get_next_rmap_item(&page); > if (!rmap_item) > return; > + if (rmap_item == ERR_PTR(-EAGAIN)) { > + /* > + * A non-zero page was skipped under > + * only_merge_zero_pages: it was examined but not > + * merged, so count it as scanned and keep going. > + */ Similarly schloppy. > + ksm_pages_scanned++; > + continue; > + } > cmp_and_merge_page(page, rmap_item); > put_page(page); > ksm_pages_scanned++; > -- > 2.34.1 > All in all adding far too much complexity in a cycle which is fix-focused... -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages 2026-11-10 10:05 ` Lorenzo Stoakes (ARM) @ 2026-11-10 11:22 ` Xu Xin (vivo) 2026-11-10 12:42 ` Lorenzo Stoakes (ARM) 0 siblings, 1 reply; 10+ messages in thread From: Xu Xin (vivo) @ 2026-11-10 11:22 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: akpm, chengming.zhou, corbet, david, link, linux-doc, linux-kernel, linux-kselftest, linux-mm, odys.yuan, rppt, surenb, tujinjiang, vbabka, xiao.wang On Tue, Nov 10, 2026 at 10:05:20AM +0000, Lorenzo Stoakes (ARM) wrote: > On Tue, Nov 10, 2026 at 03:44:49PM +0800, Xu Xin (vivo) wrote: > > When only_merge_zero_pages is enabled, ksmd only ever merges empty > > (zeroed) anonymous pages with the kernel zero page. A page that carries > > data can never be merged, yet the current code still allocates an > > rmap_item for it in scan_get_next_rmap_item() before cmp_and_merge_page() > > decides to skip it. > > > > On workloads with a large amount of non-mergeable anonymous memory this > > is a real cost: every scanned non-zero page pays for an rmap_item > > allocation, and the rmap_items accumulate in the per-mm pending list. > > For example, a MySQL instance with hundreds of gigabytes of anonymous > > memory (mostly buffer pool pages that cannot be merged) ends up with > > ksmd creating rmap_item metadata for each candidate page; the metadata > > overhead can wipe out any gain from the zero-page merging and even turn > > the net benefit negative. > > > > Add should_skip_non_zero_page(), called in scan_get_next_rmap_item() before > > get_next_rmap_item(), which, when only_merge_zero_pages is enabled, > > checks whether the candidate page is all zeros. A non-zero page is > > skipped entirely: no rmap_item is allocated for it, and any stale > > pending rmap_items below it are freed (as get_next_rmap_item() would). > > The page is reported back to ksm_do_scan() as ERR_PTR(-EAGAIN), which > > counts it as scanned and continues, so the pages_to_scan budget is > > preserved. > > > > The all-zero check (pages_identical() against the kernel zero page) reads > > the whole page. For a non-zero page this is a net win: the extra page read > > is cheaper than the rmap_item allocation and the stable/unstable tree > > operations it would otherwise trigger. > > > > A page that is zero now but is written before cmp_and_merge_page() runs > > is still handled correctly, because the checksum-based decision in > > cmp_and_merge_page() (added by the previous patch) is the authoritative > > one. > > > > Please re-write this in your own words. > > It's rambling on and never gets to the point of _what this patch does and > why_. > > I'm not interested in reading a novel to understand the code :) > > It seems very LLM generated, LLMs are _terrible_ at writing commit messages > and commits, please re-write all of that. Indeed, the description was generated by LLM, but I thought that was clear enough to present what this patch do and why. Aparrently, you don't like that style, I'll rewrite that by handd. The goal is only to avoid rmap_item allocation for pages that can never merge under only_merge_zero_pages. The implementation is simple. I'll send a v3 with those changes. > > > Signed-off-by: Xu Xin (vivo) <xu.xin@linux.dev> > > Co-developed-by: Jinjiang Tu <tujinjiang@huawei.com> > > Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> > > Why are this and the other patches in-reply-to 1/4? That's not how you send > a series in mm. That was my mistake > > > --- > > mm/ksm.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 82 insertions(+) > > > > diff --git a/mm/ksm.c b/mm/ksm.c > > index 49f3728a3d03..e641c37ee096 100644 > > --- a/mm/ksm.c > > +++ b/mm/ksm.c > > @@ -2535,6 +2535,69 @@ static bool should_skip_rmap_item(struct folio *folio, > > return true; > > } > > > > +/* > > + * Determines whether a candidate page should be skipped entirely when > > + * only_merge_zero_pages is enabled, before an rmap_item is allocated for > > + * it. > > + * > > + * A page that is not all zeros carries data and can never be merged with > > + * the kernel zero page, so there is no point in allocating an rmap_item > > + * for it. Skipping it here avoids the rmap_item metadata cost that full > > + * KSM would otherwise pay for every non-mergeable page. > > + * > > + * @rmap_list: head of the per-mm pending rmap_item list > > + * @addr: address of the candidate page > > + * @folio: folio containing the candidate page > > + * @page: the candidate page > > + * > > + * Returns true if the page should be skipped (and any stale pending > > + * rmap_items below @addr are freed), false if it should be processed. > > + */ > > This is a really weird mix of kdoc style + not kdoc style while not being a > kdoc, for a static function? > Will simplify it. > > +static bool should_skip_non_zero_page(struct ksm_rmap_item **rmap_list, > > + unsigned long addr, struct folio *folio, > > + struct page *page) > > +{ > > + struct ksm_rmap_item *rmap_item; > > + > > + if (!ksm_only_merge_zero_pages) > > + return false; > > + > > + /* > > + * Pages already merged by KSM are handled by cmp_and_merge_page(); > > + * do not skip them here. > > + */ > > + if (folio_test_ksm(folio)) > > + return false; > > + > > + /* > > + * An all-zero page is exactly what we want to merge with the kernel > > + * zero page; process it. > > + */ > > + if (pages_identical(page, ZERO_PAGE(0))) > > + return false; > > + > > + /* > > + * The page carries data: free any pending rmap_items that are below > > + * it (they are stale, as get_next_rmap_item() would), and skip it so > > + * no rmap_item is allocated for it. > > + * > > + * Compare the masked address, unlike get_next_rmap_item() which > > + * compares the raw rmap_item->address (which may carry the > > + * UNSTABLE/STABLE_FLAG bits). That is intentional: a pending > > + * rmap_item at the same address as a non-zero page is stale and must > > + * be freed here, not returned for processing. > > + */ > > We don't need a prose version of the code, and it reads like AI slop, we > don't want AI slop comments, thanks. Sorry, this is to clarify the difference between get_next_rmap_item() and here. Maybe too verbose to you ? I can simplify those words. > > > + while (*rmap_list) { > > + rmap_item = *rmap_list; > > Why is rmap_item function-scoped when it could be block-scoped here... You are right. Will fix it. > > > + if ((rmap_item->address & PAGE_MASK) > addr) > > + break; > > + *rmap_list = rmap_item->rmap_list; > > + remove_rmap_item_from_tree(rmap_item); > > + free_rmap_item(rmap_item); > > + } > > This code is very ugly. > > Newline... > > > + return true; > > > > +} > > + > > struct ksm_next_page_arg { > > struct folio *folio; > > struct page *page; > > @@ -2737,6 +2800,16 @@ static struct ksm_rmap_item *scan_get_next_rmap_item(struct page **page) > > if (tmp_page) { > > flush_anon_page(vma, tmp_page, ksm_scan.address); > > flush_dcache_page(tmp_page); > > + > > + if (should_skip_non_zero_page(ksm_scan.rmap_list, > > + ksm_scan.address, folio, > > + tmp_page)) { > > + folio_put(folio); > > + ksm_scan.address += PAGE_SIZE; > > + mmap_read_unlock(mm); > > + return ERR_PTR(-EAGAIN); > > + } > > + > > I don't love just shoving more logic into an already massive function. No > to this. > > At some point technical debt has to be paid, clean up this code first please. Emmm... I agree scan_get_next_rmap_item() is already too large and needs cleanup. But it is the main KSM scan path, and a refactor there is high-risk. The cleanup deserves a separate series with careful review and testing. > > > rmap_item = get_next_rmap_item(mm_slot, > > ksm_scan.rmap_list, ksm_scan.address); > > if (rmap_item) { > > @@ -2839,6 +2912,15 @@ static void ksm_do_scan(unsigned int scan_npages) > > rmap_item = scan_get_next_rmap_item(&page); > > if (!rmap_item) > > return; > > + if (rmap_item == ERR_PTR(-EAGAIN)) { > > + /* > > + * A non-zero page was skipped under > > + * only_merge_zero_pages: it was examined but not > > + * merged, so count it as scanned and keep going. > > + */ > > Similarly schloppy. > > > + ksm_pages_scanned++; > > + continue; > > + } > > cmp_and_merge_page(page, rmap_item); > > put_page(page); > > ksm_pages_scanned++; > > -- > > 2.34.1 > > > > All in all adding far too much complexity in a cycle which is > fix-focused... > > -- > Cheers, Lorenzo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages 2026-11-10 11:22 ` Xu Xin (vivo) @ 2026-11-10 12:42 ` Lorenzo Stoakes (ARM) 0 siblings, 0 replies; 10+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-11-10 12:42 UTC (permalink / raw) To: Xu Xin (vivo) Cc: akpm, chengming.zhou, corbet, david, link, linux-doc, linux-kernel, linux-kselftest, linux-mm, odys.yuan, rppt, surenb, tujinjiang, vbabka, xiao.wang On Tue, Nov 10, 2026 at 07:22:06PM +0800, Xu Xin (vivo) wrote: > On Tue, Nov 10, 2026 at 10:05:20AM +0000, Lorenzo Stoakes (ARM) wrote: > > On Tue, Nov 10, 2026 at 03:44:49PM +0800, Xu Xin (vivo) wrote: > > > When only_merge_zero_pages is enabled, ksmd only ever merges empty > > > (zeroed) anonymous pages with the kernel zero page. A page that carries > > > data can never be merged, yet the current code still allocates an > > > rmap_item for it in scan_get_next_rmap_item() before cmp_and_merge_page() > > > decides to skip it. > > > > > > On workloads with a large amount of non-mergeable anonymous memory this > > > is a real cost: every scanned non-zero page pays for an rmap_item > > > allocation, and the rmap_items accumulate in the per-mm pending list. > > > For example, a MySQL instance with hundreds of gigabytes of anonymous > > > memory (mostly buffer pool pages that cannot be merged) ends up with > > > ksmd creating rmap_item metadata for each candidate page; the metadata > > > overhead can wipe out any gain from the zero-page merging and even turn > > > the net benefit negative. > > > > > > Add should_skip_non_zero_page(), called in scan_get_next_rmap_item() before > > > get_next_rmap_item(), which, when only_merge_zero_pages is enabled, > > > checks whether the candidate page is all zeros. A non-zero page is > > > skipped entirely: no rmap_item is allocated for it, and any stale > > > pending rmap_items below it are freed (as get_next_rmap_item() would). > > > The page is reported back to ksm_do_scan() as ERR_PTR(-EAGAIN), which > > > counts it as scanned and continues, so the pages_to_scan budget is > > > preserved. > > > > > > The all-zero check (pages_identical() against the kernel zero page) reads > > > the whole page. For a non-zero page this is a net win: the extra page read > > > is cheaper than the rmap_item allocation and the stable/unstable tree > > > operations it would otherwise trigger. > > > > > > A page that is zero now but is written before cmp_and_merge_page() runs > > > is still handled correctly, because the checksum-based decision in > > > cmp_and_merge_page() (added by the previous patch) is the authoritative > > > one. > > > > > > > Please re-write this in your own words. > > > > It's rambling on and never gets to the point of _what this patch does and > > why_. > > > > I'm not interested in reading a novel to understand the code :) > > > > It seems very LLM generated, LLMs are _terrible_ at writing commit messages > > and commits, please re-write all of that. > > Indeed, the description was generated by LLM, but I thought that was clear enough > to present what this patch do and why. Aparrently, you don't like that style, I'll > rewrite that by handd. In general we never want LLM-generated commit messages/comments. I do however obviously appreciate and empathise with using it to assist non-native speakers. So that's fine, but ideally you would write it up yourself and then have the LLM do simply a polishing layer, keeping the length as small as possible, which should give us the best of both worlds. Thanks :) > > The goal is only to avoid rmap_item allocation for pages that can never merge > under only_merge_zero_pages. The implementation is simple. > > I'll send a v3 with those changes. Well let's wait on David's POV on this series for this cycle before a respin first thanks! > > > > > > Signed-off-by: Xu Xin (vivo) <xu.xin@linux.dev> > > > Co-developed-by: Jinjiang Tu <tujinjiang@huawei.com> > > > Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> > > > > Why are this and the other patches in-reply-to 1/4? That's not how you send > > a series in mm. > > That was my mistake Ack, sure I've made all sorts of mistakes like that myself :) > > > > > --- > > > mm/ksm.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 82 insertions(+) > > > > > > diff --git a/mm/ksm.c b/mm/ksm.c > > > index 49f3728a3d03..e641c37ee096 100644 > > > --- a/mm/ksm.c > > > +++ b/mm/ksm.c > > > @@ -2535,6 +2535,69 @@ static bool should_skip_rmap_item(struct folio *folio, > > > return true; > > > } > > > > > > +/* > > > + * Determines whether a candidate page should be skipped entirely when > > > + * only_merge_zero_pages is enabled, before an rmap_item is allocated for > > > + * it. > > > + * > > > + * A page that is not all zeros carries data and can never be merged with > > > + * the kernel zero page, so there is no point in allocating an rmap_item > > > + * for it. Skipping it here avoids the rmap_item metadata cost that full > > > + * KSM would otherwise pay for every non-mergeable page. > > > + * > > > + * @rmap_list: head of the per-mm pending rmap_item list > > > + * @addr: address of the candidate page > > > + * @folio: folio containing the candidate page > > > + * @page: the candidate page > > > + * > > > + * Returns true if the page should be skipped (and any stale pending > > > + * rmap_items below @addr are freed), false if it should be processed. > > > + */ > > > > This is a really weird mix of kdoc style + not kdoc style while not being a > > kdoc, for a static function? > > > > Will simplify it. Thanks. > > > > +static bool should_skip_non_zero_page(struct ksm_rmap_item **rmap_list, > > > + unsigned long addr, struct folio *folio, > > > + struct page *page) > > > +{ > > > + struct ksm_rmap_item *rmap_item; > > > + > > > + if (!ksm_only_merge_zero_pages) > > > + return false; > > > + > > > + /* > > > + * Pages already merged by KSM are handled by cmp_and_merge_page(); > > > + * do not skip them here. > > > + */ > > > + if (folio_test_ksm(folio)) > > > + return false; > > > + > > > + /* > > > + * An all-zero page is exactly what we want to merge with the kernel > > > + * zero page; process it. > > > + */ > > > + if (pages_identical(page, ZERO_PAGE(0))) > > > + return false; > > > + > > > + /* > > > + * The page carries data: free any pending rmap_items that are below > > > + * it (they are stale, as get_next_rmap_item() would), and skip it so > > > + * no rmap_item is allocated for it. > > > + * > > > + * Compare the masked address, unlike get_next_rmap_item() which > > > + * compares the raw rmap_item->address (which may carry the > > > + * UNSTABLE/STABLE_FLAG bits). That is intentional: a pending > > > + * rmap_item at the same address as a non-zero page is stale and must > > > + * be freed here, not returned for processing. > > > + */ > > > > We don't need a prose version of the code, and it reads like AI slop, we > > don't want AI slop comments, thanks. > > Sorry, this is to clarify the difference between get_next_rmap_item() and here. > Maybe too verbose to you ? I can simplify those words. It is too verbose yes, and effectively writes out the code in comment form, this is not helpful. > > > > > > + while (*rmap_list) { > > > + rmap_item = *rmap_list; > > > > Why is rmap_item function-scoped when it could be block-scoped here... > > You are right. Will fix it. Thanks! > > > > > > + if ((rmap_item->address & PAGE_MASK) > addr) > > > + break; > > > + *rmap_list = rmap_item->rmap_list; > > > + remove_rmap_item_from_tree(rmap_item); > > > + free_rmap_item(rmap_item); > > > + } > > > > This code is very ugly. > > > > Newline... In general I'd prefer a rethink though to make it cleaner. > > > > > + return true; > > > > > > > +} > > > + > > > struct ksm_next_page_arg { > > > struct folio *folio; > > > struct page *page; > > > @@ -2737,6 +2800,16 @@ static struct ksm_rmap_item *scan_get_next_rmap_item(struct page **page) > > > if (tmp_page) { > > > flush_anon_page(vma, tmp_page, ksm_scan.address); > > > flush_dcache_page(tmp_page); > > > + > > > + if (should_skip_non_zero_page(ksm_scan.rmap_list, > > > + ksm_scan.address, folio, > > > + tmp_page)) { > > > + folio_put(folio); > > > + ksm_scan.address += PAGE_SIZE; > > > + mmap_read_unlock(mm); > > > + return ERR_PTR(-EAGAIN); > > > + } > > > + > > > > I don't love just shoving more logic into an already massive function. No > > to this. > > > > At some point technical debt has to be paid, clean up this code first please. > > Emmm... I agree scan_get_next_rmap_item() is already too large and needs cleanup. > > But it is the main KSM scan path, and a refactor there is high-risk. The cleanup > deserves a separate series with careful review and testing. Completely disagree. By this logic we'd never improve anything, and even if it were true, it's valid to say there should be a feature freeze until that's fixed. Anyway, let's work through this a bit. At 50,000 ft: static struct ksm_rmap_item *scan_get_next_rmap_item(struct page **page) { ... if (mm_slot == &ksm_mm_head) { ... huge block ... next_mm: ... of course, nasty non-linear goto logic ... } for_each_vma(vmi, vma) { ... while (ksm_scan.address < vma->vm_end) { ... if (tmp_page) { ... <-- you propose to add open-coded logic in the most nested bit. } ... } ... } ... huge amounts of other code ... } Now very clearly you can separate out these blocks into static functions with no risk at all and eliminate most of this indentation. Digging into your proposal: if (tmp_page) { flush_anon_page(vma, tmp_page, ksm_scan.address); flush_dcache_page(tmp_page); + if (should_skip_non_zero_page(ksm_scan.rmap_list, + ksm_scan.address, folio, + tmp_page)) { + folio_put(folio); + ksm_scan.address += PAGE_SIZE; + mmap_read_unlock(mm); + return ERR_PTR(-EAGAIN); + } rmap_item = get_next_rmap_item(mm_slot, ksm_scan.rmap_list, ksm_scan.address); if (rmap_item) { ksm_scan.rmap_list = &rmap_item->rmap_list; if (should_skip_rmap_item(folio, rmap_item)) { folio_put(folio); goto next_page; } ksm_scan.address += PAGE_SIZE; *page = tmp_page; } else { folio_put(folio); } mmap_read_unlock(mm); return rmap_item; } Firstly, in context, this is: ... if (tmp_page) { ... } next_page: ksm_scan.address += PAGE_SIZE; cond_resched(); Right? So this could be: if (!tmp_page) goto next_page; ... logic in block above ... With zero risk and immediately you remove one level of indentation. Since you're returning rmap_item anyway you could do: if (should_skip_non_zero_page(ksm_scan.rmap_list, ksm_scan.address, folio, tmp_page)) { ksm_scan.address += PAGE_SIZE; rmap_item = ERR_PTR(-EAGAIN); } else { rmap_item = get_next_rmap_item(mm_slot, ksm_scan.rmap_list, ksm_scan.address); } Then: if (!IS_ERR_OR_NULL(rmap_item)) { ... } else { folio_put(folio); } mmap_read_unlock(mm); return rmap_item; Which does the mmap_read_unlock(), address update and return for you. IMO none of this is risky, and this isn't even getting into bigger refactorings you could do. Adding a few of these kinds of things as patches early in the series really isn't that hard and improves things for everybody. I'd love for this to be an instinctive thing people did when adding new functionality to stuff like this. That way we pay down technical debt as we go :) -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] selftests/ksm: consolidate ksm_stat readers into ksm_get_self_stat() 2026-11-10 7:44 ` [PATCH 1/4] ksm: add only_merge_zero_pages to merge only empty anonymous pages Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages Xu Xin (vivo) @ 2026-11-10 7:44 ` Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 4/4] selftests/ksm: add test for only_merge_zero_pages Xu Xin (vivo) 2 siblings, 0 replies; 10+ messages in thread From: Xu Xin (vivo) @ 2026-11-10 7:44 UTC (permalink / raw) To: xu.xin Cc: akpm, chengming.zhou, corbet, david, link, linux-doc, linux-kernel, linux-kselftest, linux-mm, ljs, odys.yuan, rppt, surenb, tujinjiang, vbabka, xiao.wang ksm_get_self_zero_pages() and ksm_get_self_merging_pages() both read /proc/self/ksm_stat and differ only in the key they look up. Replace them with a single ksm_get_self_stat(key) helper in vm_util.c, which reads one value from /proc/self/ksm_stat by key name and accepts only the keys used by the tests. This is a pure refactor with no functional change; it prepares for a new test that also needs to read ksm_rmap_items from /proc/self/ksm_stat. No functional change intended. Signed-off-by: Xu Xin (vivo) <xu.xin@linux.dev> --- .../selftests/mm/ksm_functional_tests.c | 18 ++++----- tools/testing/selftests/mm/vm_util.c | 40 ++++++------------- tools/testing/selftests/mm/vm_util.h | 3 +- 3 files changed, 22 insertions(+), 39 deletions(-) diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c index 135191f99962..2ffb97ae3120 100644 --- a/tools/testing/selftests/mm/ksm_functional_tests.c +++ b/tools/testing/selftests/mm/ksm_functional_tests.c @@ -85,7 +85,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot, return err_map; } - if (ksm_get_self_merging_pages() > 0) { + if (ksm_get_self_stat("ksm_merging_pages") > 0) { ksft_print_msg("Still pages merged\n"); return err_map; } @@ -144,7 +144,7 @@ static char *__mmap_and_merge_range(char val, unsigned long size, int prot, * Check if anything was merged at all. Ignore the zero page that is * accounted differently (depending on kernel support). */ - if (val && !ksm_get_self_merging_pages()) { + if (val && !ksm_get_self_stat("ksm_merging_pages")) { ksft_print_msg("No pages got merged\n"); goto unmap; } @@ -204,7 +204,7 @@ static void test_unmerge_zero_pages(void) ksft_print_msg("[RUN] %s\n", __func__); - if (ksm_get_self_zero_pages() < 0) { + if (ksm_get_self_stat("ksm_zero_pages") < 0) { ksft_test_result_skip("accessing \"/proc/self/ksm_stat\" failed\n"); return; } @@ -221,7 +221,7 @@ static void test_unmerge_zero_pages(void) /* Check if ksm_zero_pages is updated correctly after KSM merging */ pages_expected = size / pagesize; - if (pages_expected != ksm_get_self_zero_pages()) { + if (pages_expected != ksm_get_self_stat("ksm_zero_pages")) { ksft_test_result_fail("'ksm_zero_pages' updated after merging\n"); goto unmap; } @@ -234,7 +234,7 @@ static void test_unmerge_zero_pages(void) /* Check if ksm_zero_pages is updated correctly after unmerging */ pages_expected /= 2; - if (pages_expected != ksm_get_self_zero_pages()) { + if (pages_expected != ksm_get_self_stat("ksm_zero_pages")) { ksft_test_result_fail("'ksm_zero_pages' updated after unmerging\n"); goto unmap; } @@ -244,7 +244,7 @@ static void test_unmerge_zero_pages(void) *((unsigned int *)&map[offs]) = offs; /* Now we should have no zeropages remaining. */ - if (ksm_get_self_zero_pages()) { + if (ksm_get_self_stat("ksm_zero_pages")) { ksft_test_result_fail("'ksm_zero_pages' updated after write fault\n"); goto unmap; } @@ -671,7 +671,7 @@ static void test_fork_ksm_merging_page_count(void) child_pid = fork(); if (!child_pid) { init_global_file_handles(); - exit(ksm_get_self_merging_pages()); + exit(ksm_get_self_stat("ksm_merging_pages")); } else if (child_pid < 0) { ksft_test_result_fail("fork() failed\n"); goto unmap; @@ -771,7 +771,7 @@ static void test_merge_aged_pairs(void) goto unmap; } - ksft_test_result(ksm_get_self_merging_pages() == 2 * pairs, + ksft_test_result(ksm_get_self_stat("ksm_merging_pages") == 2 * pairs, "Pages aged by smart scan got merged\n"); unmap: ksm_stop(); @@ -793,7 +793,7 @@ static void init_global_file_handles(void) pagemap_fd = open("/proc/self/pagemap", O_RDONLY); if (pagemap_fd < 0) ksft_exit_skip("open(\"/proc/self/pagemap\") failed\n"); - if (ksm_get_self_merging_pages() < 0) + if (ksm_get_self_stat("ksm_merging_pages") < 0) ksft_exit_skip("accessing \"/proc/self/ksm_merging_pages\") failed\n"); pages_to_scan_fd = open("/sys/kernel/mm/ksm/pages_to_scan", O_RDWR); diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c index f26f9e4bfdbe..8f1ee1f81e36 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -904,14 +904,20 @@ bool detect_huge_zeropage(void) return enabled; } -long ksm_get_self_zero_pages(void) +long ksm_get_self_stat(const char *key) { int proc_self_ksm_stat_fd; char buf[200]; - char *substr_ksm_zero; + char *substr; size_t value_pos; ssize_t read_size; + /* Only the keys actually used by the tests are accepted. */ + if (strcmp(key, "ksm_rmap_items") && + strcmp(key, "ksm_zero_pages") && + strcmp(key, "ksm_merging_pages")) + return -EINVAL; + proc_self_ksm_stat_fd = open("/proc/self/ksm_stat", O_RDONLY); if (proc_self_ksm_stat_fd < 0) return -errno; @@ -925,34 +931,12 @@ long ksm_get_self_zero_pages(void) buf[read_size] = 0; - substr_ksm_zero = strstr(buf, "ksm_zero_pages"); - if (!substr_ksm_zero) + substr = strstr(buf, key); + if (!substr) return 0; - value_pos = strcspn(substr_ksm_zero, "0123456789"); - return strtol(substr_ksm_zero + value_pos, NULL, 10); -} - -long ksm_get_self_merging_pages(void) -{ - int proc_self_ksm_merging_pages_fd; - char buf[10]; - ssize_t ret; - - proc_self_ksm_merging_pages_fd = open("/proc/self/ksm_merging_pages", - O_RDONLY); - if (proc_self_ksm_merging_pages_fd < 0) - return -errno; - - ret = pread(proc_self_ksm_merging_pages_fd, buf, sizeof(buf) - 1, 0); - close(proc_self_ksm_merging_pages_fd); - if (ret < 0) - return -errno; - if (!ret) - return -EIO; - buf[ret] = 0; - - return strtol(buf, NULL, 10); + value_pos = strcspn(substr, "0123456789"); + return strtol(substr + value_pos, NULL, 10); } long ksm_get_full_scans(void) diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index ab1325dc6580..2b25a9eab30c 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -159,8 +159,7 @@ static inline int sz2ord(size_t size, size_t pagesize) void *sys_mremap(void *old_address, unsigned long old_size, unsigned long new_size, int flags, void *new_address); -long ksm_get_self_zero_pages(void); -long ksm_get_self_merging_pages(void); +long ksm_get_self_stat(const char *key); long ksm_get_full_scans(void); int ksm_wait_full_scans(long nr); int ksm_get_smart_scan(void); -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] selftests/ksm: add test for only_merge_zero_pages 2026-11-10 7:44 ` [PATCH 1/4] ksm: add only_merge_zero_pages to merge only empty anonymous pages Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 3/4] selftests/ksm: consolidate ksm_stat readers into ksm_get_self_stat() Xu Xin (vivo) @ 2026-11-10 7:44 ` Xu Xin (vivo) 2 siblings, 0 replies; 10+ messages in thread From: Xu Xin (vivo) @ 2026-11-10 7:44 UTC (permalink / raw) To: xu.xin Cc: akpm, chengming.zhou, corbet, david, link, linux-doc, linux-kernel, linux-kselftest, linux-mm, ljs, odys.yuan, rppt, surenb, tujinjiang, vbabka, xiao.wang Add a functional test that enables use_zero_pages and only_merge_zero_pages, maps a 2 MiB region with the first half filled with non-zero data and the second half with zeroes, and verifies that after KSM scanning reaches steady state: - ksm_zero_pages equals the zero half (merged with the kernel zero page) - ksm_merging_pages is 0 (non-zero pages never reach the stable tree) - ksm_rmap_items is 0 (non-zero pages are skipped before rmap_item allocation, and the zero pages' rmap_items are freed once a scan finds the merged pages are no longer anonymous) The test skips if the only_merge_zero_pages attribute is absent. Signed-off-by: Xu Xin (vivo) <xu.xin@linux.dev> --- .../selftests/mm/ksm_functional_tests.c | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c index 2ffb97ae3120..764ead3425cb 100644 --- a/tools/testing/selftests/mm/ksm_functional_tests.c +++ b/tools/testing/selftests/mm/ksm_functional_tests.c @@ -257,6 +257,122 @@ static void test_unmerge_zero_pages(void) munmap(map, size); } +/* + * Enable or disable /sys/kernel/mm/ksm/only_merge_zero_pages. Returns 0 on + * success, -errno on failure. + */ +static int ksm_set_only_merge_zero_pages(int enable) +{ + int fd; + ssize_t ret; + + fd = open("/sys/kernel/mm/ksm/only_merge_zero_pages", O_RDWR); + if (fd < 0) + return -errno; + + ret = write(fd, enable ? "1" : "0", 1); + close(fd); + return ret == 1 ? 0 : -errno; +} + +static void test_only_merge_zero_pages(void) +{ + const unsigned int size = 2 * MiB; + const unsigned int half = size / 2; + const unsigned int half_pages = half / pagesize; + char *map; + int i; + long zero_pages, merging_pages, rmap_items; + + ksft_print_msg("[RUN] %s\n", __func__); + + if (ksm_use_zero_pages() < 0) { + ksft_test_result_skip("write /sys/kernel/mm/ksm/use_zero_pages failed\n"); + return; + } + if (ksm_set_only_merge_zero_pages(1) < 0) { + ksft_test_result_skip("write /sys/kernel/mm/ksm/only_merge_zero_pages failed\n"); + return; + } + + /* Stabilize accounting by disabling KSM completely. */ + if (ksm_stop() < 0) { + ksft_test_result_fail("Disabling (unmerging) KSM failed\n"); + goto restore; + } + + map = mmap(NULL, size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANON, -1, 0); + if (map == MAP_FAILED) { + ksft_test_result_fail("mmap() failed\n"); + goto restore; + } + + /* Don't use THP. Ignore if THP are not around on a kernel. */ + if (madvise(map, size, MADV_NOHUGEPAGE) && errno != EINVAL) { + ksft_test_result_fail("MADV_NOHUGEPAGE failed\n"); + goto unmap; + } + + /* + * Fill the first half with non-zero data (all 0x01) and the second + * half with zeroes. Under only_merge_zero_pages the zero half should + * be merged with the kernel zero page, while the non-zero half must be + * skipped entirely (no rmap_item allocated, no stable-tree insertion). + */ + memset(map, 0x01, half); + memset(map + half, 0x00, half); + + if (madvise(map, size, MADV_MERGEABLE)) { + ksft_test_result_fail("MADV_MERGEABLE failed\n"); + goto unmap; + } + + /* + * Run KSM until steady state. Each ksm_start() waits for two full + * scans: the first records the checksums, the second merges the zero + * half with the kernel zero page. The second ksm_start() waits for + * more scans so that the rmap_items of the (now merged, no longer + * anonymous) zero pages are freed and ksm_rmap_items settles at 0. + */ + for (i = 0; i < 2; i++) { + if (ksm_start() < 0) { + ksft_test_result_fail("Running KSM failed\n"); + goto unmap; + } + } + + /* + * Expected results (steady state): + * - ksm_zero_pages: the zero half (size/2 pages) is merged with the + * kernel zero page. + * - ksm_merging_pages: 0, because the non-zero half is skipped before + * it can reach the stable tree, and zero pages are accounted in + * ksm_zero_pages, not ksm_merging_pages. + * - ksm_rmap_items: 0. The non-zero half is skipped before + * get_next_rmap_item(), so it never allocates an rmap_item; and the + * zero half's rmap_items are freed once a scan finds the (now + * merged) pages are no longer anonymous. + */ + zero_pages = ksm_get_self_stat("ksm_zero_pages"); + merging_pages = ksm_get_self_stat("ksm_merging_pages"); + rmap_items = ksm_get_self_stat("ksm_rmap_items"); + + ksft_print_msg("ksm_zero_pages=%ld ksm_merging_pages=%ld ksm_rmap_items=%ld" + " (expected %u/0/0)\n", zero_pages, merging_pages, rmap_items, + half_pages); + + ksft_test_result(zero_pages == half_pages && merging_pages == 0 && + rmap_items == 0, "only_merge_zero_pages: zero half merged, " + "non-zero half skipped\n"); + +unmap: + ksm_stop(); + munmap(map, size); +restore: + ksm_set_only_merge_zero_pages(0); +} + static void test_unmerge_discarded(void) { const unsigned int size = 2 * MiB; @@ -827,6 +943,7 @@ int main(int argc, char **argv) test_unmerge(); test_unmerge_zero_pages(); + test_only_merge_zero_pages(); test_unmerge_discarded(); #ifdef __NR_userfaultfd test_unmerge_uffd_wp(); -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] KSM: introduce only_merge_zero_pages 2026-11-10 7:43 [PATCH v2 0/4] KSM: introduce only_merge_zero_pages Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 1/4] ksm: add only_merge_zero_pages to merge only empty anonymous pages Xu Xin (vivo) @ 2026-11-10 10:08 ` Lorenzo Stoakes (ARM) 2026-11-10 11:34 ` Xu Xin (vivo) 1 sibling, 1 reply; 10+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-11-10 10:08 UTC (permalink / raw) To: Xu Xin (vivo) Cc: David Hildenbrand, Chengming Zhou, Andrew Morton, Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Jonathan Corbet, Jinjiang Tu, Qiliang Yuan, linux-mm, linux-doc, linux-kernel, linux-kselftest, Huan Yang, Wang Xiao All-in-all this seems to be adding more complexity to a current major AI bugfix target, in a cycle where we're not really looking for significant new functionality. So given [0] I wonder if this could be deferred to the next cycle? David, what do you think? In any case the code is pretty rough from a scan (I reviewed one of the commits to have a mini-dig) and looks very LLM-generated. It needs _human_ polishing before we could take it regardless. And additionally, you are shoving yet more logic into already horribly massive functions in dire need of rework. Sorry but the technical debt does have to be paid at some point, so I think this would need some initial patches doing something about that before the rest of it anyway. [0]:https://lore.kernel.org/linux-mm/a2f584f6-e382-4876-aa6d-d5a34de6ee59@kernel.org/ -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] KSM: introduce only_merge_zero_pages 2026-11-10 10:08 ` [PATCH v2 0/4] KSM: introduce only_merge_zero_pages Lorenzo Stoakes (ARM) @ 2026-11-10 11:34 ` Xu Xin (vivo) 0 siblings, 0 replies; 10+ messages in thread From: Xu Xin (vivo) @ 2026-11-10 11:34 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: David Hildenbrand, Chengming Zhou, Andrew Morton, Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Jonathan Corbet, Jinjiang Tu, Qiliang Yuan, linux-mm, linux-doc, linux-kernel, linux-kselftest, Huan Yang, Wang Xiao On Tue, Nov 10, 2026 at 10:08:00AM +0000, Lorenzo Stoakes (ARM) wrote: > All-in-all this seems to be adding more complexity to a current major AI bugfix > target, in a cycle where we're not really looking for significant new > functionality. > > So given [0] I wonder if this could be deferred to the next cycle? David, what > do you think? > > In any case the code is pretty rough from a scan (I reviewed one of the commits > to have a mini-dig) and looks very LLM-generated. > > It needs _human_ polishing before we could take it regardless. I don't mind deferring this to the next cycle. Also, the patches themselves are all written by Jinjiang and me; some of the descriptions may have been translated or generated with LLM, which helps improve efficiency for non-native English speakers like me. If you don't like that style, I'll try to avoid it and write the commit messages manually. Thanks. > > And additionally, you are shoving yet more logic into already horribly massive > functions in dire need of rework. > > Sorry but the technical debt does have to be paid at some point, so I think this > would need some initial patches doing something about that before the rest of it > anyway. Fine, I'll try but it's not that easy to refactor scan_get_next_rmap_item(). > > [0]:https://lore.kernel.org/linux-mm/a2f584f6-e382-4876-aa6d-d5a34de6ee59@kernel.org/ > > -- > Cheers, Lorenzo ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-10 12:42 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-10 7:43 [PATCH v2 0/4] KSM: introduce only_merge_zero_pages Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 1/4] ksm: add only_merge_zero_pages to merge only empty anonymous pages Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 2/4] ksm: skip rmap_item allocation for non-zero pages under only_merge_zero_pages Xu Xin (vivo) 2026-11-10 10:05 ` Lorenzo Stoakes (ARM) 2026-11-10 11:22 ` Xu Xin (vivo) 2026-11-10 12:42 ` Lorenzo Stoakes (ARM) 2026-11-10 7:44 ` [PATCH 3/4] selftests/ksm: consolidate ksm_stat readers into ksm_get_self_stat() Xu Xin (vivo) 2026-11-10 7:44 ` [PATCH 4/4] selftests/ksm: add test for only_merge_zero_pages Xu Xin (vivo) 2026-11-10 10:08 ` [PATCH v2 0/4] KSM: introduce only_merge_zero_pages Lorenzo Stoakes (ARM) 2026-11-10 11:34 ` Xu Xin (vivo)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®