mm: honor FGP_NOWAIT for page cache page allocation
Subsystems (2)
PATCH v2 1/1 mm: honor FGP_NOWAIT for page cache page allocation
If we're creating a page cache page with FGP_CREAT but FGP_NOWAIT is set, we should dial back the gfp flags to avoid frivolous blocking which is trivial to hit in low memory conditions:
[ 10.117661] __schedule+0x8c/0x550 [ 10.118305] schedule+0x58/0xa0 [ 10.118897] schedule_timeout+0x30/0xdc [ 10.119610] __wait_for_common+0x88/0x114 [ 10.120348] wait_for_completion+0x1c/0x24 [ 10.121103] __flush_work.isra.0+0x16c/0x19c [ 10.121896] flush_work+0xc/0x14 [ 10.122496] __drain_all_pages+0x144/0x218 [ 10.123267] drain_all_pages+0x10/0x18 [ 10.123941] __alloc_pages+0x464/0x9e4 [ 10.124633] __folio_alloc+0x18/0x3c [ 10.125294] __filemap_get_folio+0x17c/0x204 [ 10.126084] iomap_write_begin+0xf8/0x428 [ 10.126829] iomap_file_buffered_write+0x144/0x24c [ 10.127710] xfs_file_buffered_write+0xe8/0x248 [ 10.128553] xfs_file_write_iter+0xa8/0x120 [ 10.129324] io_write+0x16c/0x38c [ 10.129940] io_issue_sqe+0x70/0x1cc [ 10.130617] io_queue_sqe+0x18/0xfc [ 10.131277] io_submit_sqes+0x5d4/0x600 [ 10.131946] __arm64_sys_io_uring_enter+0x224/0x600 [ 10.132752] invoke_syscall.constprop.0+0x70/0xc0 [ 10.133616] do_el0_svc+0xd0/0x118 [ 10.134238] el0_svc+0x78/0xa0
Clear IO, FS, and reclaim flags and mark the allocation as GFP_NOWAIT and add __GFP_NOWARN to avoid polluting dmesg with pointless allocations failures. A caller with FGP_NOWAIT must be expected to handle the resulting -EAGAIN return and retry from a suitable context without NOWAIT set.
---
V2:
- Improve commit message
- Add GFP_NOWAIT to allow kswapd wakeup
mm/filemap.c | 4 ++++ 1 file changed, 4 insertions(+)
mm/filemap.c +4 -0
@@ -1988,6 +1988,10 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, gfp |= __GFP_WRITE;if (fgp_flags &FGP_NOFS ) gfp &= ~__GFP_FS; +if (fgp_flags &FGP_NOWAIT ) { + gfp &= ~GFP_KERNEL ; + gfp |=GFP_NOWAIT | __GFP_NOWARN; + } folio =filemap_alloc_folio (gfp,0 );if (!folio)
Signature · 4 lines
2.35.1 -- Jens Axboe
Quoted 29 lines · Jens Axboe
If we're creating a page cache page with
FGP_CREATbutFGP_NOWAITis set, we should dial back the gfp flags to avoid frivolous blocking which is trivial to hit in low memory conditions:[ 10.117661] __schedule+0x8c/0x550 [ 10.118305] schedule+0x58/0xa0 [ 10.118897] schedule_timeout+0x30/0xdc [ 10.119610] __wait_for_common+0x88/0x114 [ 10.120348] wait_for_completion+0x1c/0x24 [ 10.121103] __flush_work.isra.0+0x16c/0x19c [ 10.121896] flush_work+0xc/0x14 [ 10.122496] __drain_all_pages+0x144/0x218 [ 10.123267] drain_all_pages+0x10/0x18 [ 10.123941] __alloc_pages+0x464/0x9e4 [ 10.124633] __folio_alloc+0x18/0x3c [ 10.125294] __filemap_get_folio+0x17c/0x204 [ 10.126084] iomap_write_begin+0xf8/0x428 [ 10.126829] iomap_file_buffered_write+0x144/0x24c [ 10.127710] xfs_file_buffered_write+0xe8/0x248 [ 10.128553] xfs_file_write_iter+0xa8/0x120 [ 10.129324] io_write+0x16c/0x38c [ 10.129940] io_issue_sqe+0x70/0x1cc [ 10.130617] io_queue_sqe+0x18/0xfc [ 10.131277] io_submit_sqes+0x5d4/0x600 [ 10.131946] __arm64_sys_io_uring_enter+0x224/0x600 [ 10.132752] invoke_syscall.constprop.0+0x70/0xc0 [ 10.133616] do_el0_svc+0xd0/0x118 [ 10.134238] el0_svc+0x78/0xa0
Quoted 7 lines
Clear IO, FS, and reclaim flags and mark the allocation as
GFP_NOWAITand add__GFP_NOWARNto avoid polluting dmesg with pointless allocations failures. A caller withFGP_NOWAITmust be expected to handle the resulting-EAGAINreturn and retry from a suitable context without NOWAIT set.Signed-off-by Jens Axboe <[email protected]>
Matthew Wilcox
maintainermm/filemap.c struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, · line 1994
Quoted 9 lines · Jens Axboe
+++
b/mm/filemap.cmm/filemap.c +4
-0@@ -1988,6 +1988,10 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, gfp |= __GFP_WRITE;if (fgp_flags &FGP_NOFS ) gfp &= ~__GFP_FS; +if (fgp_flags &FGP_NOWAIT ) { + gfp &= ~GFP_KERNEL ; + gfp |=GFP_NOWAIT | __GFP_NOWARN; + }
Wouldn't it be simpler to write:
if (fgp_flags & FGP_NOWAIT) {
gfp &= ~__GFP_DIRECT_RECLAIM;
gfp |= __GFP_NOWARN;
}(I'd prefer it if the caller passed in the GFP flags that it actually wants, but looking at the patches that prompted this, that seems rather annoying to do)
Quoted 13 lines · Matthew Wilcox
mm/filemap.c struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, · line 1994
Quoted 9 lines · Jens Axboe
+++
b/mm/filemap.cmm/filemap.c +4
-0@@ -1988,6 +1988,10 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, gfp |= __GFP_WRITE;if (fgp_flags &FGP_NOFS ) gfp &= ~__GFP_FS; +if (fgp_flags &FGP_NOWAIT ) { + gfp &= ~GFP_KERNEL ; + gfp |=GFP_NOWAIT | __GFP_NOWARN; + }Wouldn't it be simpler to write:
Quoted 4 lines
if (fgp_flags & FGP_NOWAIT) {
gfp &= ~__GFP_DIRECT_RECLAIM;
gfp |= __GFP_NOWARN;
}That won't clear IO or FS, though? We really just want GFP_NOWAIT | __GFP_NOWARN here, but didn't want to make assumptions on whatever else would be set in gfp already.
Quoted 3 lines
(I'd prefer it if the caller passed in the GFP flags that it actually wants, but looking at the patches that prompted this, that seems rather annoying to do)
Yes me too, but that's a lot of annoying churn...
Signature · 1 line
Jens Axboe