From: Matthew Wilcox <willy@infradead.org>
To: Hugh Dickins <hughd@google.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>,
akpm@linux-foundation.org, david@redhat.com,
wangkefeng.wang@huawei.com, chrisl@kernel.org,
ying.huang@intel.com, 21cnbao@gmail.com, ryan.roberts@arm.com,
shy828301@gmail.com, ziy@nvidia.com, ioworker0@gmail.com,
da.gomez@samsung.com, p.raghav@samsung.com, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 4/9] mm: filemap: use xa_get_order() to get the swap entry order
Date: Mon, 26 Aug 2024 00:28:56 +0100 [thread overview]
Message-ID: <Zsu-OCxgB9OAK050@casper.infradead.org> (raw)
In-Reply-To: <d3dc75e2-40a7-8439-734c-19d83707164c@google.com>
On Sun, Aug 25, 2024 at 02:55:41PM -0700, Hugh Dickins wrote:
> The second issue is that swap is more slippery to work with than
> folios or pages: in the folio_nr_pages() case, that number is stable
> because we hold a refcount (which stops a THP from being split), and
> later we'll be taking folio lock too. None of that in the swap case,
> so (depending on how a large entry gets split) the xa_get_order() result
> is not reliable. Likewise for other uses of xa_get_order() in this series.
>
> There needs to be some kind of locking or retry to make the order usable,
> and to avoid shmem_free_swap() occasionally freeing more than it ought.
> I'll give it thought after.
My original thought was that we'd take a bit from the swap entry in
order to indicate the order of the entry. I was surprised to see the
xa_get_order() implementation, but didn't remember why it wouldn't work.
Sorry.
Anyway, that's how I think it should be fixed. Is that enough? Holding
a reference on the folio prevents truncation, splitting, and so on.
There's no reference to be held on a swap entry, so could we have some
moderately implausible series of operations while holding only the RCU
read lock that would cause us to go wrong?
next prev parent reply other threads:[~2024-08-25 23:29 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-12 7:42 [PATCH v5 0/9] support large folio swap-out and swap-in for shmem Baolin Wang
2024-08-12 7:42 ` [PATCH v5 1/9] mm: swap: extend swap_shmem_alloc() to support batch SWAP_MAP_SHMEM flag setting Baolin Wang
2024-08-12 7:42 ` [PATCH v5 2/9] mm: shmem: extend shmem_partial_swap_usage() to support large folio swap Baolin Wang
2024-08-12 7:42 ` [PATCH v5 3/9] mm: shmem: return number of pages beeing freed in shmem_free_swap Baolin Wang
2024-08-12 7:42 ` [PATCH v5 4/9] mm: filemap: use xa_get_order() to get the swap entry order Baolin Wang
2024-08-25 21:55 ` Hugh Dickins
2024-08-25 23:28 ` Matthew Wilcox [this message]
2024-08-27 10:10 ` Baolin Wang
2024-08-29 8:07 ` Hugh Dickins
2024-08-29 12:40 ` Baolin Wang
2024-08-30 10:18 ` Hugh Dickins
2024-08-12 7:42 ` [PATCH v5 5/9] mm: shmem: use swap_free_nr() to free shmem swap entries Baolin Wang
2024-08-12 7:42 ` [PATCH v5 6/9] mm: shmem: support large folio allocation for shmem_replace_folio() Baolin Wang
2024-08-25 22:05 ` Hugh Dickins
2024-08-27 3:06 ` Baolin Wang
2024-08-12 7:42 ` [PATCH v5 7/9] mm: shmem: drop folio reference count using 'nr_pages' in shmem_delete_from_page_cache() Baolin Wang
2024-08-12 7:42 ` [PATCH v5 8/9] mm: shmem: split large entry if the swapin folio is not large Baolin Wang
2024-08-25 22:31 ` Hugh Dickins
2024-08-27 6:46 ` Baolin Wang
2024-08-12 7:42 ` [PATCH v5 9/9] mm: shmem: support large folio swap out Baolin Wang
2024-08-25 23:14 ` Hugh Dickins
2024-08-27 6:58 ` Baolin Wang
2024-08-28 8:28 ` [PATCH] mm: shmem: support large folio swap out fix 2 Baolin Wang
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=Zsu-OCxgB9OAK050@casper.infradead.org \
--to=willy@infradead.org \
--cc=21cnbao@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=baolin.wang@linux.alibaba.com \
--cc=chrisl@kernel.org \
--cc=da.gomez@samsung.com \
--cc=david@redhat.com \
--cc=hughd@google.com \
--cc=ioworker0@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=p.raghav@samsung.com \
--cc=ryan.roberts@arm.com \
--cc=shy828301@gmail.com \
--cc=wangkefeng.wang@huawei.com \
--cc=ying.huang@intel.com \
--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;
as well as URLs for NNTP newsgroup(s).