mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kiryl Shutsemau <kirill@shutemov.name>
To: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 David Hildenbrand <david@kernel.org>,
	Lorenzo Stoakes <ljs@kernel.org>,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org,
	kernel-team@meta.com, Zi Yan <ziy@nvidia.com>,
	 "Liam R . Howlett" <liam@infradead.org>,
	Nico Pache <nico.pache@linux.dev>,
	 Ryan Roberts <ryan.roberts@arm.com>, Dev Jain <dev.jain@arm.com>,
	Barry Song <baohua@kernel.org>,
	 Lance Yang <lance.yang@linux.dev>,
	Usama Arif <usama.arif@linux.dev>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Jann Horn <jannh@google.com>
Subject: Re: [PATCH 05/12] mm/collapse: state what a collapse may do in the policy
Date: Mon, 7 Sep 2026 11:56:00 +0100	[thread overview]
Message-ID: <ap6XERs2j7e1lZCU@thinkstation> (raw)
In-Reply-To: <c070e67a-4c1e-478f-aa28-09fb88f8a786@linux.alibaba.com>

On Mon, Sep 07, 2026 at 05:05:19PM +0800, Baolin Wang wrote:
> > +/* What a collapse is allowed to do, decided by the caller that asks for it */
> > +struct collapse_policy {
> > +	/* Limits, stated per PMD; HPAGE_PMD_NR means "no limit" */
> > +	unsigned int max_ptes_none;
> > +	unsigned int max_ptes_swap;
> > +	unsigned int max_ptes_shared;
> > +
> > +	/*
> > +	 * Hold a sub-PMD window to a stricter rule than a PMD: no swapped-out
> > +	 * and no shared PTEs at all, and max_ptes_none as
> > +	 * collapse_max_ptes_none() scales it.
> > +	 */
> > +	bool strict_sub_pmd;
> 
> This is a bit confusing to me. Actually, the check for mTHP collapse is
> stricter.
> 
> How about naming it 'allow_mthp_collapse'? That way we can keep the most
> original comments for the collapse_max_ptes_xxx() functions, which is
> clearer to me.
> 
> If others have a better name, please ignore my comment.

I would rather keep strict_sub_pmd.

Which orders a caller asks for is already decided elsewhere:
collapse_possible_orders() hands khugepaged every anonymous order and
MADV_COLLAPSE the PMD order only, from tva_type.

A flag called allow_mthp_collapse next to that would read as
a second place deciding the same thing, and flipping it would not change
which orders get collapsed.  What it does change is how a sub-PMD window
is judged once one is asked for, and it is read at exactly the three
places that judge one.

I will do:

      /* Take no swapped-out or shared PTE into a sub-PMD collapse */
      bool strict_sub_pmd;

> 
> > +	/*
> > +	 * Collapse only where it looks worth doing: require some sign the
> > +	 * range is in use, and leave clean lazyfree folios for reclaim rather
> > +	 * than collapsing them into a folio that is not lazyfree.
> > +	 */
> > +	bool skip_lazyfree;
> > +	bool require_referenced;
> > +
> > +	/*
> > +	 * Finish the job rather than leaving it half done for a fault to pick
> > +	 * up: map the PMD over a file collapse before returning, and write
> > +	 * dirty pages back and retry once instead of refusing them.  Both cost
> > +	 * latency the caller has to be willing to pay.
> > +	 */
> 
> Can you simplify theses comments? I think the 'install_pmd' is easy to
> expalain. :)

Done for v2.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

  reply	other threads:[~2026-09-07 10:56 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 15:10 [PATCH 00/12] mm/collapse: separate a collapse from its callers Kiryl Shutsemau
2026-09-04 15:10 ` [PATCH 01/12] mm/khugepaged: drop redundant mm_struct pin in madvise_collapse() Kiryl Shutsemau
2026-09-04 15:58   ` Zi Yan
2026-09-07  7:33   ` Baolin Wang
2026-09-04 15:10 ` [PATCH 02/12] mm/khugepaged: count collapses where khugepaged makes them Kiryl Shutsemau
2026-09-05  2:25   ` Zi Yan
2026-09-07  7:40   ` Baolin Wang
2026-09-04 15:10 ` [PATCH 03/12] mm/khugepaged: rename mthp_present_ptes bitmap to eligible_ptes Kiryl Shutsemau
2026-09-05  2:28   ` Zi Yan
2026-09-07  7:54   ` Baolin Wang
2026-09-07 10:35     ` Kiryl Shutsemau
2026-09-04 15:10 ` [PATCH 04/12] mm/collapse: add collapse.h for the collapse interface Kiryl Shutsemau
2026-09-05  2:36   ` Zi Yan
2026-09-07 10:41     ` Kiryl Shutsemau
2026-09-07  8:04   ` Baolin Wang
2026-09-04 15:10 ` [PATCH 05/12] mm/collapse: state what a collapse may do in the policy Kiryl Shutsemau
2026-09-05  2:44   ` Zi Yan
2026-09-07 10:49     ` Kiryl Shutsemau
2026-09-07 19:40       ` Zi Yan
2026-09-07  9:05   ` Baolin Wang
2026-09-07 10:56     ` Kiryl Shutsemau [this message]
2026-09-08  1:48       ` Baolin Wang
2026-09-04 15:10 ` [PATCH 06/12] mm/collapse: drop the collapse_possible() wrapper Kiryl Shutsemau
2026-09-05  2:45   ` Zi Yan
2026-09-07  8:28   ` Baolin Wang
2026-09-04 15:10 ` [PATCH 07/12] mm/collapse: name the per-table scan reset for what it resets Kiryl Shutsemau
2026-09-05 18:05   ` Zi Yan
2026-09-07  8:31   ` Baolin Wang
2026-09-04 15:10 ` [PATCH 08/12] mm/collapse: separate scanning a PTE table from collapsing it Kiryl Shutsemau
2026-09-06  2:30   ` Zi Yan
2026-09-07 11:34     ` Kiryl Shutsemau
2026-09-04 15:10 ` [PATCH 09/12] mm/collapse: open-code collapse_single_pmd() in its two callers Kiryl Shutsemau
2026-09-04 15:10 ` [PATCH 10/12] mm/collapse: work out the orders a VMA allows once per VMA Kiryl Shutsemau
2026-09-04 15:10 ` [PATCH 11/12] mm/collapse: declare the collapse interface in collapse.h Kiryl Shutsemau
2026-09-04 15:10 ` [PATCH 12/12] mm/collapse: implement MADV_COLLAPSE in madvise.c Kiryl Shutsemau
2026-09-06  0:23 ` [PATCH 00/12] mm/collapse: separate a collapse from its callers Andrew Morton
2026-09-07 10:28   ` Kiryl Shutsemau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ap6XERs2j7e1lZCU@thinkstation \
    --to=kirill@shutemov.name \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=jannh@google.com \
    --cc=kernel-team@meta.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=nico.pache@linux.dev \
    --cc=ryan.roberts@arm.com \
    --cc=usama.arif@linux.dev \
    --cc=vbabka@kernel.org \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®