All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: zhaozhengzhuo <zhaozhengzhuo@uniontech.com>
Cc: Mike Kaplinskiy <mike@recall.ai>,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	 "Liam R . Howlett" <liam@infradead.org>,
	David Hildenbrand <david@kernel.org>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Jann Horn <jannh@google.com>
Subject: Re: [RFC PATCH v1] mm/madvise: prefetch private file COW swap entries
Date: Fri, 4 Sep 2026 15:52:06 +0100	[thread overview]
Message-ID: <aprY-BekgYlsK-Y9@gremlin> (raw)
In-Reply-To: <D758D1DD4372AF8E+20260904100919.2139680-1-zhaozhengzhuo@uniontech.com>

On Fri, Sep 04, 2026 at 06:09:19PM +0800, zhaozhengzhuo wrote:
...


> Fixes: 1998cc048901 ("mm: make madvise(MADV_WILLNEED) support swap file prefetch")

Fixes totally inappropriate.

> Reported-by: Mike Kaplinskiy <mike@recall.ai>

Missing Closes tag.

> Link: https://lore.kernel.org/linux-mm/CABeknB_S2XJSHFgnHdgnN0rjzHhH4oQJs_APq9fvxHztQ_pgiA@mail.gmail.com/
> Signed-off-by: zhaozhengzhuo <zhaozhengzhuo@uniontech.com>

(Very likely) missing Assisted-by tag.

Please follow kernel procedure.

https://docs.kernel.org/process/coding-assistants.html
https://docs.kernel.org/process/generated-content.html

I already sent at patch at
https://lore.kernel.org/linux-mm/apm68kgHC0NK2zVI@gremlin/ before this one, so
I'm going to take care of this thanks.

> ---
>  mm/madvise.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 73c2901b9adb..96cbce6c7f8b 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -193,10 +193,16 @@ static int madvise_update_vma(vm_flags_t new_flags,
>  }
>
>  #ifdef CONFIG_SWAP
> +struct swapin_walk_ctx {
> +	struct vm_area_struct *vma;
> +	bool swapped;
> +};

This is silly.

> +
>  static int swapin_walk_pmd_entry(pmd_t *pmd, unsigned long start,
>  		unsigned long end, struct mm_walk *walk)
>  {
> -	struct vm_area_struct *vma = walk->private;
> +	struct swapin_walk_ctx *swc = walk->private;
> +	struct vm_area_struct *vma = swc->vma;
>  	struct swap_io_ctx ctx = {};
>  	pte_t *ptep = NULL;
>  	spinlock_t *ptl;
> @@ -223,8 +229,10 @@ static int swapin_walk_pmd_entry(pmd_t *pmd, unsigned long start,
>
>  		folio = read_swap_cache_async(&ctx, entry, GFP_HIGHUSER_MOVABLE,
>  					vma, addr);
> -		if (folio)
> +		if (folio) {
> +			swc->swapped = true;
>  			folio_put(folio);
> +		}

This is pointless, it's not a good trade off.

>  	}
>
>  	if (ptep)
> @@ -297,10 +305,22 @@ static long madvise_willneed(struct madvise_behavior *madv_behavior)
>  	loff_t offset;
>
>  #ifdef CONFIG_SWAP
> -	if (!file) {
> -		walk_page_range_vma(vma, start, end, &swapin_walk_ops, vma);
> -		lru_add_drain(); /* Push any new pages onto the LRU now */
> -		return 0;
> +	bool private_file = file && !(vma->vm_flags & VM_SHARED);

This is not correct. My version uses the correct CoW predicate.

> +
> +	/*
> +	 * A private file mapping can contain anonymous COW pages. Once such
> +	 * pages are swapped out, their PTEs contain swap entries even though
> +	 * the VMA still has vm_file set. Prefetch those pages as well; file
> +	 * readahead can only fetch the original file contents.
> +	 */
> +	if (!file || (private_file && vma->anon_vma)) {
> +		struct swapin_walk_ctx ctx = { .vma = vma };
> +
> +		walk_page_range_vma(vma, start, end, &swapin_walk_ops, &ctx);
> +		if (ctx.swapped)
> +			lru_add_drain(); /* Push any new pages onto the LRU now */
> +		if (!file)
> +			return 0;

General structure is OK, but again I already sent a patch.

So please no v2, I will handle this thanks!

>  	}
>
>  	if (shmem_mapping(file->f_mapping)) {
> --
> 2.43.0

--
Cheers, Lorenzo


  reply	other threads:[~2026-09-04 14:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 18:24 [BUG/RFC] mm/madvise: MADV_WILLNEED skips swapped file-backed COW pages Mike Kaplinskiy
2026-09-03  7:05 ` zhaozhengzhuo
2026-09-03 17:03   ` Mike Kaplinskiy
2026-09-04 10:09     ` [RFC PATCH v1] mm/madvise: prefetch private file COW swap entries zhaozhengzhuo
2026-09-04 14:52       ` Lorenzo Stoakes (ARM) [this message]
2026-09-05 11:37         ` zhaozhengzhuo
2026-09-03 19:03 ` [BUG/RFC] mm/madvise: MADV_WILLNEED skips swapped file-backed COW pages Lorenzo Stoakes (ARM)

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=aprY-BekgYlsK-Y9@gremlin \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=jannh@google.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike@recall.ai \
    --cc=vbabka@kernel.org \
    --cc=zhaozhengzhuo@uniontech.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.