* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-10 17:39 ` [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently Zi Yan
@ 2025-10-10 18:02 ` Luis Chamberlain
2025-10-13 17:11 ` Zi Yan
2025-10-11 2:25 ` Lance Yang
` (4 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Luis Chamberlain @ 2025-10-10 18:02 UTC (permalink / raw)
To: Zi Yan
Cc: linmiaohe, david, jane.chu, kernel, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, nao.horiguchi, Lorenzo Stoakes,
Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
> folios.
>
> Fix it by failing a split if the folio cannot be split to the target order.
>
> 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@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
> Signed-off-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Luis
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-10 18:02 ` Luis Chamberlain
@ 2025-10-13 17:11 ` Zi Yan
0 siblings, 0 replies; 22+ messages in thread
From: Zi Yan @ 2025-10-13 17:11 UTC (permalink / raw)
To: Luis Chamberlain
Cc: linmiaohe, david, jane.chu, kernel, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, nao.horiguchi, Lorenzo Stoakes,
Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On 10 Oct 2025, at 14:02, Luis Chamberlain wrote:
> On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
>> folios.
>>
>> Fix it by failing a split if the folio cannot be split to the target order.
>>
>> 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@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Thanks.
--
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-10 17:39 ` [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently Zi Yan
2025-10-10 18:02 ` Luis Chamberlain
@ 2025-10-11 2:25 ` Lance Yang
2025-10-13 17:06 ` Zi Yan
2025-10-11 9:00 ` kernel test robot
` (3 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Lance Yang @ 2025-10-11 2:25 UTC (permalink / raw)
To: Zi Yan
Cc: akpm, syzkaller-bugs, mcgrof, nao.horiguchi, Lorenzo Stoakes,
kernel, Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts,
jane.chu, Dev Jain, Barry Song, Matthew Wilcox (Oracle),
linux-fsdevel, david, linux-kernel, linux-mm, linmiaohe,
syzbot+e6367ea2fdab6ed46056
On 2025/10/11 01:39, 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 really get min_order_for_split() order
> folios.
>
> Fix it by failing a split if the folio cannot be split to the target order.
>
> 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@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
> include/linux/huge_mm.h | 28 +++++-----------------------
> mm/huge_memory.c | 9 +--------
> mm/truncate.c | 6 ++++--
> 3 files changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 8eec7a2a977b..9950cda1526a 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -394,34 +394,16 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
> * Return: 0: split is successful, otherwise split failed.
> */
> static inline int try_folio_split(struct folio *folio, struct page *page,
> - struct list_head *list)
> + struct list_head *list, unsigned int order)
Seems like we need to add the order parameter to the stub for
try_folio_split() as well?
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
...
#else /* CONFIG_TRANSPARENT_HUGEPAGE */
static inline int try_folio_split(struct folio *folio, struct page *page,
struct list_head *list)
{
VM_WARN_ON_ONCE_FOLIO(1, folio);
return -EINVAL;
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
Cheers,
Lance
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-11 2:25 ` Lance Yang
@ 2025-10-13 17:06 ` Zi Yan
0 siblings, 0 replies; 22+ messages in thread
From: Zi Yan @ 2025-10-13 17:06 UTC (permalink / raw)
To: Lance Yang
Cc: akpm, syzkaller-bugs, mcgrof, nao.horiguchi, Lorenzo Stoakes,
kernel, Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts,
jane.chu, Dev Jain, Barry Song, Matthew Wilcox (Oracle),
linux-fsdevel, david, linux-kernel, linux-mm, linmiaohe,
syzbot+e6367ea2fdab6ed46056
On 10 Oct 2025, at 22:25, Lance Yang wrote:
> On 2025/10/11 01:39, 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 really get min_order_for_split() order
>> folios.
>>
>> Fix it by failing a split if the folio cannot be split to the target order.
>>
>> 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@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>> include/linux/huge_mm.h | 28 +++++-----------------------
>> mm/huge_memory.c | 9 +--------
>> mm/truncate.c | 6 ++++--
>> 3 files changed, 10 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index 8eec7a2a977b..9950cda1526a 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -394,34 +394,16 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
>> * Return: 0: split is successful, otherwise split failed.
>> */
>> static inline int try_folio_split(struct folio *folio, struct page *page,
>> - struct list_head *list)
>> + struct list_head *list, unsigned int order)
>
> Seems like we need to add the order parameter to the stub for try_folio_split() as well?
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>
> ...
>
> #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> static inline int try_folio_split(struct folio *folio, struct page *page,
> struct list_head *list)
> {
> VM_WARN_ON_ONCE_FOLIO(1, folio);
> return -EINVAL;
> }
>
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
Thanks. It is also reported by lkp robot. Will fix it.
--
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-10 17:39 ` [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently Zi Yan
2025-10-10 18:02 ` Luis Chamberlain
2025-10-11 2:25 ` Lance Yang
@ 2025-10-11 9:00 ` kernel test robot
2025-10-12 0:41 ` Wei Yang
` (2 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2025-10-11 9:00 UTC (permalink / raw)
To: Zi Yan, linmiaohe, david, jane.chu, kernel,
syzbot+e6367ea2fdab6ed46056, syzkaller-bugs
Cc: oe-kbuild-all, ziy, akpm, mcgrof, nao.horiguchi, Lorenzo Stoakes,
Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
Hi Zi,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.17 next-20251010]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zi-Yan/mm-huge_memory-do-not-change-split_huge_page-target-order-silently/20251011-014145
base: linus/master
patch link: https://lore.kernel.org/r/20251010173906.3128789-2-ziy%40nvidia.com
patch subject: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
config: parisc-allnoconfig (https://download.01.org/0day-ci/archive/20251011/202510111633.onu4Yaey-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251011/202510111633.onu4Yaey-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510111633.onu4Yaey-lkp@intel.com/
All errors (new ones prefixed by >>):
mm/truncate.c: In function 'truncate_inode_partial_folio':
>> mm/truncate.c:229:14: error: too many arguments to function 'try_folio_split'; expected 3, have 4
229 | if (!try_folio_split(folio, split_at, NULL, min_order)) {
| ^~~~~~~~~~~~~~~ ~~~~~~~~~
In file included from include/linux/mm.h:1081,
from arch/parisc/include/asm/cacheflush.h:5,
from include/linux/cacheflush.h:5,
from include/linux/highmem.h:8,
from include/linux/bvec.h:10,
from include/linux/blk_types.h:10,
from include/linux/writeback.h:13,
from include/linux/backing-dev.h:16,
from mm/truncate.c:12:
include/linux/huge_mm.h:588:19: note: declared here
588 | static inline int try_folio_split(struct folio *folio, struct page *page,
| ^~~~~~~~~~~~~~~
mm/truncate.c:259:25: error: too many arguments to function 'try_folio_split'; expected 3, have 4
259 | try_folio_split(folio2, split_at2, NULL, min_order);
| ^~~~~~~~~~~~~~~ ~~~~~~~~~
include/linux/huge_mm.h:588:19: note: declared here
588 | static inline int try_folio_split(struct folio *folio, struct page *page,
| ^~~~~~~~~~~~~~~
vim +/try_folio_split +229 mm/truncate.c
179
180 /*
181 * Handle partial folios. The folio may be entirely within the
182 * range if a split has raced with us. If not, we zero the part of the
183 * folio that's within the [start, end] range, and then split the folio if
184 * it's large. split_page_range() will discard pages which now lie beyond
185 * i_size, and we rely on the caller to discard pages which lie within a
186 * newly created hole.
187 *
188 * Returns false if splitting failed so the caller can avoid
189 * discarding the entire folio which is stubbornly unsplit.
190 */
191 bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
192 {
193 loff_t pos = folio_pos(folio);
194 size_t size = folio_size(folio);
195 unsigned int offset, length;
196 struct page *split_at, *split_at2;
197 unsigned int min_order;
198
199 if (pos < start)
200 offset = start - pos;
201 else
202 offset = 0;
203 if (pos + size <= (u64)end)
204 length = size - offset;
205 else
206 length = end + 1 - pos - offset;
207
208 folio_wait_writeback(folio);
209 if (length == size) {
210 truncate_inode_folio(folio->mapping, folio);
211 return true;
212 }
213
214 /*
215 * We may be zeroing pages we're about to discard, but it avoids
216 * doing a complex calculation here, and then doing the zeroing
217 * anyway if the page split fails.
218 */
219 if (!mapping_inaccessible(folio->mapping))
220 folio_zero_range(folio, offset, length);
221
222 if (folio_needs_release(folio))
223 folio_invalidate(folio, offset, length);
224 if (!folio_test_large(folio))
225 return true;
226
227 min_order = mapping_min_folio_order(folio->mapping);
228 split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
> 229 if (!try_folio_split(folio, split_at, NULL, min_order)) {
230 /*
231 * try to split at offset + length to make sure folios within
232 * the range can be dropped, especially to avoid memory waste
233 * for shmem truncate
234 */
235 struct folio *folio2;
236
237 if (offset + length == size)
238 goto no_split;
239
240 split_at2 = folio_page(folio,
241 PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
242 folio2 = page_folio(split_at2);
243
244 if (!folio_try_get(folio2))
245 goto no_split;
246
247 if (!folio_test_large(folio2))
248 goto out;
249
250 if (!folio_trylock(folio2))
251 goto out;
252
253 /*
254 * make sure folio2 is large and does not change its mapping.
255 * Its split result does not matter here.
256 */
257 if (folio_test_large(folio2) &&
258 folio2->mapping == folio->mapping)
259 try_folio_split(folio2, split_at2, NULL, min_order);
260
261 folio_unlock(folio2);
262 out:
263 folio_put(folio2);
264 no_split:
265 return true;
266 }
267 if (folio_test_dirty(folio))
268 return false;
269 truncate_inode_folio(folio->mapping, folio);
270 return true;
271 }
272
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-10 17:39 ` [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently Zi Yan
` (2 preceding siblings ...)
2025-10-11 9:00 ` kernel test robot
@ 2025-10-12 0:41 ` Wei Yang
2025-10-13 17:07 ` Zi Yan
2025-10-12 8:24 ` Pankaj Raghav (Samsung)
2025-10-15 14:25 ` Lorenzo Stoakes
5 siblings, 1 reply; 22+ messages in thread
From: Wei Yang @ 2025-10-12 0:41 UTC (permalink / raw)
To: Zi Yan
Cc: linmiaohe, david, jane.chu, kernel, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, mcgrof, nao.horiguchi, Lorenzo Stoakes,
Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
>folios.
>
>Fix it by failing a split if the folio cannot be split to the target order.
>
>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@syzkaller.appspotmail.com
>Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
>Signed-off-by: Zi Yan <ziy@nvidia.com>
>---
> include/linux/huge_mm.h | 28 +++++-----------------------
> mm/huge_memory.c | 9 +--------
> mm/truncate.c | 6 ++++--
> 3 files changed, 10 insertions(+), 33 deletions(-)
>
>diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>index 8eec7a2a977b..9950cda1526a 100644
>--- a/include/linux/huge_mm.h
>+++ b/include/linux/huge_mm.h
>@@ -394,34 +394,16 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
> * Return: 0: split is successful, otherwise split failed.
> */
It is better to update the document of try_folio_split()
> static inline int try_folio_split(struct folio *folio, struct page *page,
>- struct list_head *list)
>+ struct list_head *list, unsigned int order)
> {
>- int ret = min_order_for_split(folio);
>-
>- if (ret < 0)
>- return ret;
>-
>- if (!non_uniform_split_supported(folio, 0, false))
>+ if (!non_uniform_split_supported(folio, order, false))
> return split_huge_page_to_list_to_order(&folio->page, list,
>- ret);
>- return folio_split(folio, ret, page, list);
>+ order);
>+ return folio_split(folio, order, page, list);
> }
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-12 0:41 ` Wei Yang
@ 2025-10-13 17:07 ` Zi Yan
0 siblings, 0 replies; 22+ messages in thread
From: Zi Yan @ 2025-10-13 17:07 UTC (permalink / raw)
To: Wei Yang
Cc: linmiaohe, david, jane.chu, kernel, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, mcgrof, nao.horiguchi, Lorenzo Stoakes,
Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On 11 Oct 2025, at 20:41, Wei Yang wrote:
> On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
>> folios.
>>
>> Fix it by failing a split if the folio cannot be split to the target order.
>>
>> 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@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>> include/linux/huge_mm.h | 28 +++++-----------------------
>> mm/huge_memory.c | 9 +--------
>> mm/truncate.c | 6 ++++--
>> 3 files changed, 10 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index 8eec7a2a977b..9950cda1526a 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -394,34 +394,16 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
>> * Return: 0: split is successful, otherwise split failed.
>> */
>
> It is better to update the document of try_folio_split()
Sure. Will do.
--
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-10 17:39 ` [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently Zi Yan
` (3 preceding siblings ...)
2025-10-12 0:41 ` Wei Yang
@ 2025-10-12 8:24 ` Pankaj Raghav (Samsung)
2025-10-13 17:11 ` Zi Yan
2025-10-15 14:25 ` Lorenzo Stoakes
5 siblings, 1 reply; 22+ messages in thread
From: Pankaj Raghav (Samsung) @ 2025-10-12 8:24 UTC (permalink / raw)
To: Zi Yan
Cc: linmiaohe, david, jane.chu, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, mcgrof, nao.horiguchi, Lorenzo Stoakes,
Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
> folios.
>
> Fix it by failing a split if the folio cannot be split to the target order.
>
> 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@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
LGTM with the suggested changes to the !CONFIG_THP try_folio_split().
Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-12 8:24 ` Pankaj Raghav (Samsung)
@ 2025-10-13 17:11 ` Zi Yan
0 siblings, 0 replies; 22+ messages in thread
From: Zi Yan @ 2025-10-13 17:11 UTC (permalink / raw)
To: Pankaj Raghav (Samsung)
Cc: linmiaohe, david, jane.chu, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, mcgrof, nao.horiguchi, Lorenzo Stoakes,
Baolin Wang, Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On 12 Oct 2025, at 4:24, Pankaj Raghav (Samsung) wrote:
> On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
>> folios.
>>
>> Fix it by failing a split if the folio cannot be split to the target order.
>>
>> 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@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
> LGTM with the suggested changes to the !CONFIG_THP try_folio_split().
>
> Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
Thanks.
--
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-10 17:39 ` [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently Zi Yan
` (4 preceding siblings ...)
2025-10-12 8:24 ` Pankaj Raghav (Samsung)
@ 2025-10-15 14:25 ` Lorenzo Stoakes
2025-10-15 22:57 ` Zi Yan
5 siblings, 1 reply; 22+ messages in thread
From: Lorenzo Stoakes @ 2025-10-15 14:25 UTC (permalink / raw)
To: Zi Yan
Cc: linmiaohe, david, jane.chu, kernel, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, mcgrof, nao.horiguchi, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
> folios.
>
> Fix it by failing a split if the folio cannot be split to the target order.
>
> 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@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
> Signed-off-by: Zi Yan <ziy@nvidia.com>
Generally ok with the patch in general but a bunch of comments below!
> ---
> include/linux/huge_mm.h | 28 +++++-----------------------
> mm/huge_memory.c | 9 +--------
> mm/truncate.c | 6 ++++--
> 3 files changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 8eec7a2a977b..9950cda1526a 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -394,34 +394,16 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
> * Return: 0: split is successful, otherwise split failed.
> */
You need to update the kdoc too.
Also can you mention there this is the function you should use if you want
to specify an order?
Maybe we should rename this function to try_folio_split_to_order() to make
that completely explicit now that we're making other splitting logic always
split to order-0?
> static inline int try_folio_split(struct folio *folio, struct page *page,
> - struct list_head *list)
> + struct list_head *list, unsigned int order)
Is this target order? I see non_uniform_split_supported() calls this
new_order so maybe let's use the same naming so as not to confuse it with
the current folio order?
Also - nitty one, but should we put the order as 3rd arg rather than 4th?
As it seems it's normal to pass NULL list, and it's a bit weird to see a
NULL in the middle of the args.
> {
> - int ret = min_order_for_split(folio);
> -
> - if (ret < 0)
> - return ret;
OK so the point of removing this is that we assume in truncate (the only
user) that we already have this information (i.e. from
mapping_min_folio_order()) right?
> -
> - if (!non_uniform_split_supported(folio, 0, false))
> + if (!non_uniform_split_supported(folio, order, false))
While we're here can we make the mystery meat last param commented like:
if (!non_uniform_split_supported(folio, order, /* warns= */false))
> return split_huge_page_to_list_to_order(&folio->page, list,
> - ret);
> - return folio_split(folio, ret, page, list);
> + order);
> + return folio_split(folio, order, page, list);
> }
> 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.
> - */
> - return split_huge_page_to_list_to_order(page, NULL, ret);
> + return split_huge_page_to_list_to_order(page, NULL, 0);
OK so the idea here is that callers would expect to split to 0 and the
specific instance where we would actually want this behaviour of splittnig
to a minimum order is now limited only to try_folio_split() (or
try_folio_split_to_order() if you rename)?
> }
> void deferred_split_folio(struct folio *folio, bool partially_mapped);
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 0fb4af604657..af06ee6d2206 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3829,8 +3829,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);
Why are we dropping this?
> ret = -EINVAL;
> goto out;
> }
> @@ -4173,12 +4171,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..1c15149ae8e9 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(folio, split_at, NULL, 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(folio2, split_at2, NULL, min_order);
>
> folio_unlock(folio2);
> out:
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-15 14:25 ` Lorenzo Stoakes
@ 2025-10-15 22:57 ` Zi Yan
2025-10-16 8:34 ` Lorenzo Stoakes
0 siblings, 1 reply; 22+ messages in thread
From: Zi Yan @ 2025-10-15 22:57 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linmiaohe, david, jane.chu, kernel, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, mcgrof, nao.horiguchi, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On 15 Oct 2025, at 10:25, Lorenzo Stoakes wrote:
> On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
>> folios.
>>
>> Fix it by failing a split if the folio cannot be split to the target order.
>>
>> 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@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Generally ok with the patch in general but a bunch of comments below!
>
>> ---
>> include/linux/huge_mm.h | 28 +++++-----------------------
>> mm/huge_memory.c | 9 +--------
>> mm/truncate.c | 6 ++++--
>> 3 files changed, 10 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index 8eec7a2a977b..9950cda1526a 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -394,34 +394,16 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
>> * Return: 0: split is successful, otherwise split failed.
>> */
>
> You need to update the kdoc too.
Done it locally.
>
> Also can you mention there this is the function you should use if you want
> to specify an order?
You mean min_order_for_split()? Sure.
>
> Maybe we should rename this function to try_folio_split_to_order() to make
> that completely explicit now that we're making other splitting logic always
> split to order-0?
Sure.
>
>> static inline int try_folio_split(struct folio *folio, struct page *page,
>> - struct list_head *list)
>> + struct list_head *list, unsigned int order)
>
> Is this target order? I see non_uniform_split_supported() calls this
> new_order so maybe let's use the same naming so as not to confuse it with
> the current folio order?
Sure, will rename it to new_order.
>
> Also - nitty one, but should we put the order as 3rd arg rather than 4th?
>
> As it seems it's normal to pass NULL list, and it's a bit weird to see a
> NULL in the middle of the args.
OK, will reorder the args.
>
>> {
>> - int ret = min_order_for_split(folio);
>> -
>> - if (ret < 0)
>> - return ret;
>
> OK so the point of removing this is that we assume in truncate (the only
> user) that we already have this information (i.e. from
> mapping_min_folio_order()) right?
Right.
>
>> -
>> - if (!non_uniform_split_supported(folio, 0, false))
>> + if (!non_uniform_split_supported(folio, order, false))
>
> While we're here can we make the mystery meat last param commented like:
>
> if (!non_uniform_split_supported(folio, order, /* warns= */false))
Sure.
>
>> return split_huge_page_to_list_to_order(&folio->page, list,
>> - ret);
>> - return folio_split(folio, ret, page, list);
>> + order);
>> + return folio_split(folio, order, page, list);
>> }
>> 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.
>> - */
>> - return split_huge_page_to_list_to_order(page, NULL, ret);
>> + return split_huge_page_to_list_to_order(page, NULL, 0);
>
> OK so the idea here is that callers would expect to split to 0 and the
> specific instance where we would actually want this behaviour of splittnig
> to a minimum order is now limited only to try_folio_split() (or
> try_folio_split_to_order() if you rename)?
>
Before commit e220917fa507 (the one to be fixed), split_huge_page() always
splits @page to order 0. It is just restoring the original behavior.
If caller wants to split a different order, they should use
split_huge_page_to_list_to_order() (current no such user except debugfs test
code).
>> }
>> void deferred_split_folio(struct folio *folio, bool partially_mapped);
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 0fb4af604657..af06ee6d2206 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3829,8 +3829,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);
>
> Why are we dropping this?
This is used to catch “misuse” of split_huge_page_to_list_to_order(), when
caller wants to split a LBS folio to an order smaller than
mapping_min_folio_order(). It is based on the assumption that split code
should never fail on a LBS folio. But that assumption is causing problems
like the reported memory failure one. So it is removed to allow split code
to fail without a warning if a LBS folio cannot be split to the new_order.
>
>> ret = -EINVAL;
>> goto out;
>> }
>
>> @@ -4173,12 +4171,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..1c15149ae8e9 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(folio, split_at, NULL, 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(folio2, split_at2, NULL, min_order);
>>
>> folio_unlock(folio2);
>> out:
>> --
>> 2.51.0
>>
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/2] mm/huge_memory: do not change split_huge_page*() target order silently.
2025-10-15 22:57 ` Zi Yan
@ 2025-10-16 8:34 ` Lorenzo Stoakes
0 siblings, 0 replies; 22+ messages in thread
From: Lorenzo Stoakes @ 2025-10-16 8:34 UTC (permalink / raw)
To: Zi Yan
Cc: linmiaohe, david, jane.chu, kernel, syzbot+e6367ea2fdab6ed46056,
syzkaller-bugs, akpm, mcgrof, nao.horiguchi, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Matthew Wilcox (Oracle),
linux-fsdevel, linux-kernel, linux-mm
On Wed, Oct 15, 2025 at 06:57:37PM -0400, Zi Yan wrote:
> On 15 Oct 2025, at 10:25, Lorenzo Stoakes wrote:
>
> > On Fri, Oct 10, 2025 at 01:39:05PM -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 really get min_order_for_split() order
> >> folios.
> >>
> >> Fix it by failing a split if the folio cannot be split to the target order.
> >>
> >> 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@syzkaller.appspotmail.com
> >> Closes: https://lore.kernel.org/all/68d2c943.a70a0220.1b52b.02b3.GAE@google.com/
> >> Signed-off-by: Zi Yan <ziy@nvidia.com>
> >
> > Generally ok with the patch in general but a bunch of comments below!
> >
> >> ---
> >> include/linux/huge_mm.h | 28 +++++-----------------------
> >> mm/huge_memory.c | 9 +--------
> >> mm/truncate.c | 6 ++++--
> >> 3 files changed, 10 insertions(+), 33 deletions(-)
> >>
> >> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> >> index 8eec7a2a977b..9950cda1526a 100644
> >> --- a/include/linux/huge_mm.h
> >> +++ b/include/linux/huge_mm.h
> >> @@ -394,34 +394,16 @@ static inline int split_huge_page_to_list_to_order(struct page *page, struct lis
> >> * Return: 0: split is successful, otherwise split failed.
> >> */
> >
> > You need to update the kdoc too.
>
> Done it locally.
Thanks!
>
> >
> > Also can you mention there this is the function you should use if you want
> > to specify an order?
>
> You mean min_order_for_split()? Sure.
No, I mean try_folio_split_to_order() :)
But ofc this applies to min_order_for_split() also
>
> >
> > Maybe we should rename this function to try_folio_split_to_order() to make
> > that completely explicit now that we're making other splitting logic always
> > split to order-0?
>
> Sure.
Thanks
> >
> >> static inline int try_folio_split(struct folio *folio, struct page *page,
> >> - struct list_head *list)
> >> + struct list_head *list, unsigned int order)
> >
> > Is this target order? I see non_uniform_split_supported() calls this
> > new_order so maybe let's use the same naming so as not to confuse it with
> > the current folio order?
>
> Sure, will rename it to new_order.
Thanks
>
> >
> > Also - nitty one, but should we put the order as 3rd arg rather than 4th?
> >
> > As it seems it's normal to pass NULL list, and it's a bit weird to see a
> > NULL in the middle of the args.
>
> OK, will reorder the args.
Thanks
>
> >
> >> {
> >> - int ret = min_order_for_split(folio);
> >> -
> >> - if (ret < 0)
> >> - return ret;
> >
> > OK so the point of removing this is that we assume in truncate (the only
> > user) that we already have this information (i.e. from
> > mapping_min_folio_order()) right?
>
> Right.
>
> >
> >> -
> >> - if (!non_uniform_split_supported(folio, 0, false))
> >> + if (!non_uniform_split_supported(folio, order, false))
> >
> > While we're here can we make the mystery meat last param commented like:
> >
> > if (!non_uniform_split_supported(folio, order, /* warns= */false))
>
> Sure.
Thanks
>
> >
> >> return split_huge_page_to_list_to_order(&folio->page, list,
> >> - ret);
> >> - return folio_split(folio, ret, page, list);
> >> + order);
> >> + return folio_split(folio, order, page, list);
> >> }
> >> 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.
> >> - */
> >> - return split_huge_page_to_list_to_order(page, NULL, ret);
> >> + return split_huge_page_to_list_to_order(page, NULL, 0);
> >
> > OK so the idea here is that callers would expect to split to 0 and the
> > specific instance where we would actually want this behaviour of splittnig
> > to a minimum order is now limited only to try_folio_split() (or
> > try_folio_split_to_order() if you rename)?
> >
>
> Before commit e220917fa507 (the one to be fixed), split_huge_page() always
> splits @page to order 0. It is just restoring the original behavior.
> If caller wants to split a different order, they should use
> split_huge_page_to_list_to_order() (current no such user except debugfs test
> code).
Yeah makes sense, though now they can also use try_folio_split_to_order() of
course!
>
> >> }
> >> void deferred_split_folio(struct folio *folio, bool partially_mapped);
> >>
> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> index 0fb4af604657..af06ee6d2206 100644
> >> --- a/mm/huge_memory.c
> >> +++ b/mm/huge_memory.c
> >> @@ -3829,8 +3829,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);
> >
> > Why are we dropping this?
>
> This is used to catch “misuse” of split_huge_page_to_list_to_order(), when
> caller wants to split a LBS folio to an order smaller than
> mapping_min_folio_order(). It is based on the assumption that split code
> should never fail on a LBS folio. But that assumption is causing problems
> like the reported memory failure one. So it is removed to allow split code
> to fail without a warning if a LBS folio cannot be split to the new_order.
OK fair, we shouldn't be warning if this is something that can actually
reasonably happen.
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 22+ messages in thread