| Message ID | 20240416172533.663418-1-david@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] mm/ksm: remove page_mapcount() usage in stable_tree_search() | expand |
On 4/17/24 1:25 AM, David Hildenbrand wrote: > We want to limit the use of page_mapcount() to the places where it is > absolutely necessary. > > If our folio has a stable node, it is a (small) KSM folio -- see > folio_stable_node(). Let's use folio_mapcount() in stable_tree_search() > instead, which results in no functional change. > > The mapcount > 1 check is a bit confusing, because that's usually a check > for page sharing. Looks like the reason is that we are guaranteed to not > exceed ksm_max_page_sharing for the tree KSM folio when merging with > that. Let's update the documentation to make that clearer. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Alex Shi (tencent) <alexs@kernel.org> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > mm/ksm.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/mm/ksm.c b/mm/ksm.c > index 486c9974f8e20..159604ad47799 100644 > --- a/mm/ksm.c > +++ b/mm/ksm.c > @@ -1909,12 +1909,15 @@ static struct page *stable_tree_search(struct page *page) > if (page_node) { > VM_BUG_ON(page_node->head != &migrate_nodes); > /* > - * Test if the migrated page should be merged > - * into a stable node dup. If the mapcount is > - * 1 we can migrate it with another KSM page > - * without adding it to the chain. > + * If the mapcount of our migrated KSM folio is > + * at most 1, we can merge it with another > + * KSM folio where we know that we have space > + * for one more mapping without exceeding the > + * ksm_max_page_sharing limit: see > + * chain_prune(). This way, we can avoid adding > + * this stable node to the chain. > */ > - if (page_mapcount(page) > 1) > + if (folio_mapcount(folio) > 1) > goto chain_append; > } > LGTM Reviewed-by: Alex Shi <alexs@kernel.org>
diff --git a/mm/ksm.c b/mm/ksm.c index 486c9974f8e20..159604ad47799 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -1909,12 +1909,15 @@ static struct page *stable_tree_search(struct page *page) if (page_node) { VM_BUG_ON(page_node->head != &migrate_nodes); /* - * Test if the migrated page should be merged - * into a stable node dup. If the mapcount is - * 1 we can migrate it with another KSM page - * without adding it to the chain. + * If the mapcount of our migrated KSM folio is + * at most 1, we can merge it with another + * KSM folio where we know that we have space + * for one more mapping without exceeding the + * ksm_max_page_sharing limit: see + * chain_prune(). This way, we can avoid adding + * this stable node to the chain. */ - if (page_mapcount(page) > 1) + if (folio_mapcount(folio) > 1) goto chain_append; }
We want to limit the use of page_mapcount() to the places where it is absolutely necessary. If our folio has a stable node, it is a (small) KSM folio -- see folio_stable_node(). Let's use folio_mapcount() in stable_tree_search() instead, which results in no functional change. The mapcount > 1 check is a bit confusing, because that's usually a check for page sharing. Looks like the reason is that we are guaranteed to not exceed ksm_max_page_sharing for the tree KSM folio when merging with that. Let's update the documentation to make that clearer. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Alex Shi (tencent) <alexs@kernel.org> Signed-off-by: David Hildenbrand <david@redhat.com> --- mm/ksm.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)