linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Kairui Song <ryncsn@gmail.com>
To: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: linux-mm@kvack.org, Hugh Dickins <hughd@google.com>,
	 Andrew Morton <akpm@linux-foundation.org>,
	Kemeng Shi <shikemeng@huaweicloud.com>,
	 Nhat Pham <nphamcs@gmail.com>, Chris Li <chrisl@kernel.org>,
	Baoquan He <bhe@redhat.com>,  Barry Song <baohua@kernel.org>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] mm/shmem, swap: fix race of truncate and swap entry split
Date: Tue, 13 Jan 2026 18:10:28 +0800	[thread overview]
Message-ID: <CAMgjq7DrrCx78K3uccsfpGeQfC-_+LuONSefJ+Vd+aCjyncwKw@mail.gmail.com> (raw)
In-Reply-To: <d95f9ea4-aa47-4d85-9b76-11afd0fb3ee7@linux.alibaba.com>

On Tue, Jan 13, 2026 at 3:16 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
> Hi Kairui,
>
> Sorry for late reply.

No problem, I was also quite busy with other works :)

>
> Yes, so I just mentioned your swapoff case.
>
> >> Actually, the real question is how to handle the case where a large swap
> >> entry happens to cross the 'end' when calling shmem_truncate_range(). If
> >> the shmem mapping stores a folio, we would split that large folio by
> >> truncate_inode_partial_folio(). If the shmem mapping stores a large swap
> >> entry, then as you noted, the truncation range can indeed exceed the 'end'.
> >>
> >> But with your change, that large swap entry would not be truncated, and
> >> I’m not sure whether that might cause other issues. Perhaps the best
> >> approach is to first split the large swap entry and only truncate the
> >> swap entries within the 'end' boundary like the
> >> truncate_inode_partial_folio() does.
> >
> > Right... I was thinking that the shmem_undo_range iterates the undo
> > range twice IIUC, in the second try it will retry if shmem_free_swap
> > returns 0:
> >
> > swaps_freed = shmem_free_swap(mapping, indices[i], end - indices[i], folio);
> > if (!swaps_freed) {
> >      /* Swap was replaced by page: retry */
> >      index = indices[i];
> >      break;
> > }
> >
> > So I thought shmem_free_swap returning 0 is good enough. Which is not,
> > it may cause the second loop to retry forever.
>
> After further investigation, I think your original fix seems to be the
> right direction, as the second loop’s find_lock_entries() will filter
> out large swap entries crossing the 'end' boundary. Sorry for noise.
>
> See the code in find_lock_entries() (Thanks to Hugh:))
>
>         } else {
>                 nr = 1 << xas_get_order(&xas);
>                 base = xas.xa_index & ~(nr - 1);
>                 /* Omit order>0 value which begins before the start */
>                 if (base < *start)
>                         continue;
>                 /* Omit order>0 value which extends beyond the end */
>                 if (base + nr - 1 > end)
>                         break;
>         }
>
> Then the shmem_get_partial_folio() will swap-in the large swap entry and
> split the large folio which crosses the 'end' boundary.

Right, thanks for the info.

But what about find_get_entries under whole_folios? Even though a
large entry is splitted before that, a new large entry that crosses
`end` could appear after that and before find_get_entries, and return
by find_get_entries.

I think we could just skip large entries that cross `end` in the
second loop, since if the entry exists before truncate, it must have
been split. We can ignore newly appeared entries.

If that's OK I can send two patches, one to ignore the large entries
in the second loop, one to fix shmem_free_swap following your
suggestion in this reply.


      reply	other threads:[~2026-01-13 10:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-11 17:53 Kairui Song
2026-01-12  4:00 ` Baolin Wang
2026-01-12  5:56   ` Kairui Song
2026-01-12  8:22     ` Baolin Wang
2026-01-12  9:55       ` Kairui Song
2026-01-13  7:16         ` Baolin Wang
2026-01-13 10:10           ` Kairui Song [this message]

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=CAMgjq7DrrCx78K3uccsfpGeQfC-_+LuONSefJ+Vd+aCjyncwKw@mail.gmail.com \
    --to=ryncsn@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=bhe@redhat.com \
    --cc=chrisl@kernel.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nphamcs@gmail.com \
    --cc=shikemeng@huaweicloud.com \
    --cc=stable@vger.kernel.org \
    /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