| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <bc842ea0-3faa-4f0f-8ad9-06b6e079b4bf@lucifer.local>
Date: Tue, 21 Oct 2025 16:46:52 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Zi Yan <ziy@...dia.com>
Cc: linmiaohe@...wei.com, david@...hat.com, jane.chu@...cle.com,
kernel@...kajraghav.com,
syzbot+e6367ea2fdab6ed46056@...kaller.appspotmail.com,
syzkaller-bugs@...glegroups.com, akpm@...ux-foundation.org,
mcgrof@...nel.org, nao.horiguchi@...il.com,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Nico Pache <npache@...hat.com>, Ryan Roberts <ryan.roberts@....com>,
Dev Jain <dev.jain@....com>, Barry Song <baohua@...nel.org>,
Lance Yang <lance.yang@...ux.dev>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Wei Yang <richard.weiyang@...il.com>, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
stable@...r.kernel.org, Pankaj Raghav <p.raghav@...sung.com>
Subject: Re: [PATCH v3] mm/huge_memory: do not change split_huge_page*()
target order silently.
On Fri, Oct 17, 2025 at 07:28:49AM -0400, Zi Yan wrote:
> On 17 Oct 2025, at 5:19, Lorenzo Stoakes wrote:
>
> > On Thu, Oct 16, 2025 at 09:36:30PM -0400, Zi Yan wrote:
> >> Page cache folios from a file system that support large block size (LBS)
> >> can have minimal folio order greater than 0, thus a high order folio might
> >> not be able to be split down to order-0. Commit e220917fa507 ("mm: split a
> >> folio in minimum folio order chunks") bumps the target order of
> >> split_huge_page*() to the minimum allowed order when splitting a LBS folio.
> >> This causes confusion for some split_huge_page*() callers like memory
> >> failure handling code, since they expect after-split folios all have
> >> order-0 when split succeeds but in reality get min_order_for_split() order
> >> folios and give warnings.
> >>
> >> Fix it by failing a split if the folio cannot be split to the target order.
> >> Rename try_folio_split() to try_folio_split_to_order() to reflect the added
> >> new_order parameter. Remove its unused list parameter.
> >
> > You're not mentioning that you removed the warning here, you should do that,
> > especially as that seems to be the motive for the cc: stable...
>
> The warning I removed below is not the warning triggered by the original code.
> The one I removed never gets triggered due to the bump of target split order
> and it is removed to avoid another warning as the bump is removed by my change.
> The triggered warning is in memory_failure(), since the code assumes after
> a successful split, folios are never large.
OK that wasn't clear before thanks.
>
> >
> >>
> >> Fixes: e220917fa507 ("mm: split a folio in minimum folio order chunks")
> >> [The test poisons LBS folios, which cannot be split to order-0 folios, and
> >> also tries to poison all memory. The non split LBS folios take more memory
> >> than the test anticipated, leading to OOM. The patch fixed the kernel
> >> warning and the test needs some change to avoid OOM.]
> >> Reported-by: syzbot+e6367ea2fdab6ed46056@...kaller.appspotmail.com
> >> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
> >> Cc: stable@...r.kernel.org
> >> Signed-off-by: Zi Yan <ziy@...dia.com>
> >
> > With nits addressed above and below this functionally LGTM so:
> >
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
>
> Thanks.
>
> >
> >> Reviewed-by: Luis Chamberlain <mcgrof@...nel.org>
> >> Reviewed-by: Pankaj Raghav <p.raghav@...sung.com>
> >> Reviewed-by: Wei Yang <richard.weiyang@...il.com>
> >> ---
> >> From V2[1]:
> >> 1. Removed a typo in try_folio_split_to_order() comment.
> >> 2. Sent the Fixes patch separately.
> >
> > You really should have mentioned you split this off and the other series now
> > relies on it.
> >
> > Now it's just confusing unless you go read the other thread...
>
> OK. Will add it.
>
> >
> >>
> >> [1] https://lore.kernel.org/linux-mm/20251016033452.125479-1-ziy@nvidia.com/
> >>
> >> include/linux/huge_mm.h | 55 +++++++++++++++++------------------------
> >> mm/huge_memory.c | 9 +------
> >> mm/truncate.c | 6 +++--
> >> 3 files changed, 28 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> >> index c4a811958cda..7698b3542c4f 100644
> >> --- a/include/linux/huge_mm.h
> >> +++ b/include/linux/huge_mm.h
> >> @@ -383,45 +383,30 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
> >> }
> >>
> >> /*
> >> - * try_folio_split - try to split a @folio at @page using non uniform split.
> >> + * try_folio_split_to_order - try to split a @folio at @page to @new_order using
> >> + * non uniform split.
> >> * @folio: folio to be split
> >> - * @page: split to order-0 at the given page
> >> - * @list: store the after-split folios
> >> + * @page: split to @new_order at the given page
> >> + * @new_order: the target split order
> >> *
> >> - * Try to split a @folio at @page using non uniform split to order-0, if
> >> - * non uniform split is not supported, fall back to uniform split.
> >> + * Try to split a @folio at @page using non uniform split to @new_order, if
> >> + * non uniform split is not supported, fall back to uniform split. After-split
> >> + * folios are put back to LRU list. Use min_order_for_split() to get the lower
> >> + * bound of @new_order.
> >> *
> >> * Return: 0: split is successful, otherwise split failed.
> >> */
> >> -static inline int try_folio_split(struct folio *folio, struct page *page,
> >> - struct list_head *list)
> >> +static inline int try_folio_split_to_order(struct folio *folio,
> >> + struct page *page, unsigned int new_order)
> >
> > OK I guess you realised that every list passed here is NULL anyway?
>
> Yes.
> >
> >> {
> >> - int ret = min_order_for_split(folio);
> >> -
> >> - if (ret < 0)
> >> - return ret;
> >> -
> >> - if (!non_uniform_split_supported(folio, 0, false))
> >> - return split_huge_page_to_list_to_order(&folio->page, list,
> >> - ret);
> >> - return folio_split(folio, ret, page, list);
> >> + if (!non_uniform_split_supported(folio, new_order, /* warns= */ false))
> >> + return split_huge_page_to_list_to_order(&folio->page, NULL,
> >> + new_order);
> >> + return folio_split(folio, new_order, page, NULL);
> >> }
> >> static inline int split_huge_page(struct page *page)
> >> {
> >> - struct folio *folio = page_folio(page);
> >> - int ret = min_order_for_split(folio);
> >> -
> >> - if (ret < 0)
> >> - return ret;
> >> -
> >> - /*
> >> - * split_huge_page() locks the page before splitting and
> >> - * expects the same page that has been split to be locked when
> >> - * returned. split_folio(page_folio(page)) cannot be used here
> >> - * because it converts the page to folio and passes the head
> >> - * page to be split.
> >> - */
> >
> > Why are we deleting this comment?
>
> This comment is added because folio was used to get min_order_for_split()
> and there was a version using split_folio() on folio causing unlock bugs.
> Now folio is removed, so the comment is no longer needed.
Ack.
>
> >
> >> - return split_huge_page_to_list_to_order(page, NULL, ret);
> >> + return split_huge_page_to_list_to_order(page, NULL, 0);
> >> }
> >> void deferred_split_folio(struct folio *folio, bool partially_mapped);
> >> #ifdef CONFIG_MEMCG
> >> @@ -611,14 +596,20 @@ static inline int split_huge_page(struct page *page)
> >> return -EINVAL;
> >> }
> >>
> >> +static inline int min_order_for_split(struct folio *folio)
> >> +{
> >> + VM_WARN_ON_ONCE_FOLIO(1, folio);
> >> + return -EINVAL;
> >> +}
> >> +
> >> static inline int split_folio_to_list(struct folio *folio, struct list_head *list)
> >> {
> >> VM_WARN_ON_ONCE_FOLIO(1, folio);
> >> return -EINVAL;
> >> }
> >>
> >> -static inline int try_folio_split(struct folio *folio, struct page *page,
> >> - struct list_head *list)
> >> +static inline int try_folio_split_to_order(struct folio *folio,
> >> + struct page *page, unsigned int new_order)
> >> {
> >> VM_WARN_ON_ONCE_FOLIO(1, folio);
> >> return -EINVAL;
> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> index f14fbef1eefd..fc65ec3393d2 100644
> >> --- a/mm/huge_memory.c
> >> +++ b/mm/huge_memory.c
> >> @@ -3812,8 +3812,6 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> >>
> >> min_order = mapping_min_folio_order(folio->mapping);
> >> if (new_order < min_order) {
> >> - VM_WARN_ONCE(1, "Cannot split mapped folio below min-order: %u",
> >> - min_order);
> >> ret = -EINVAL;
> >> goto out;
> >> }
> >> @@ -4165,12 +4163,7 @@ int min_order_for_split(struct folio *folio)
> >>
> >> int split_folio_to_list(struct folio *folio, struct list_head *list)
> >> {
> >> - int ret = min_order_for_split(folio);
> >> -
> >> - if (ret < 0)
> >> - return ret;
> >> -
> >> - return split_huge_page_to_list_to_order(&folio->page, list, ret);
> >> + return split_huge_page_to_list_to_order(&folio->page, list, 0);
> >> }
> >>
> >> /*
> >> diff --git a/mm/truncate.c b/mm/truncate.c
> >> index 91eb92a5ce4f..9210cf808f5c 100644
> >> --- a/mm/truncate.c
> >> +++ b/mm/truncate.c
> >> @@ -194,6 +194,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> >> size_t size = folio_size(folio);
> >> unsigned int offset, length;
> >> struct page *split_at, *split_at2;
> >> + unsigned int min_order;
> >>
> >> if (pos < start)
> >> offset = start - pos;
> >> @@ -223,8 +224,9 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> >> if (!folio_test_large(folio))
> >> return true;
> >>
> >> + min_order = mapping_min_folio_order(folio->mapping);
> >> split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
> >> - if (!try_folio_split(folio, split_at, NULL)) {
> >> + if (!try_folio_split_to_order(folio, split_at, min_order)) {
> >> /*
> >> * try to split at offset + length to make sure folios within
> >> * the range can be dropped, especially to avoid memory waste
> >> @@ -254,7 +256,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> >> */
> >> if (folio_test_large(folio2) &&
> >> folio2->mapping == folio->mapping)
> >> - try_folio_split(folio2, split_at2, NULL);
> >> + try_folio_split_to_order(folio2, split_at2, min_order);
> >>
> >> folio_unlock(folio2);
> >> out:
> >> --
> >> 2.51.0
> >>
>
>
> --
> Best Regards,
> Yan, Zi
Powered by blists - more mailing lists