| Message ID | fda6ea77-97c5-7235-421a-21ca5b2c48f9@kernel.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: honor FGP_NOWAIT for page cache page allocation | expand |
On Fri, Jul 01, 2022 at 02:08:26PM -0600, Jens Axboe wrote: > 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 nowarn, > as FGP_NOWAIT are expected to be able to handle -EAGAIN failures and > retry from a suitable context without NOWAIT set. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > mm/filemap.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/filemap.c b/mm/filemap.c > index ffdfbc8b0e3c..9b747d3247c6 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -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; Do you think the semantic of FGP_NOWAIT should be similar to GFP_NOWAIT? That is allow kswapd wakeup. > + gfp |= __GFP_NOWARN; > + } > > folio = filemap_alloc_folio(gfp, 0); > if (!folio) > -- > 2.35.1 > > -- > Jens Axboe > >
> On Jul 1, 2022, at 6:56 PM, Shakeel Butt <shakeelb@google.com> wrote: > > On Fri, Jul 01, 2022 at 02:08:26PM -0600, Jens Axboe wrote: >> 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 nowarn, >> as FGP_NOWAIT are expected to be able to handle -EAGAIN failures and >> retry from a suitable context without NOWAIT set. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> --- >> mm/filemap.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/mm/filemap.c b/mm/filemap.c >> index ffdfbc8b0e3c..9b747d3247c6 100644 >> --- a/mm/filemap.c >> +++ b/mm/filemap.c >> @@ -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; > > Do you think the semantic of FGP_NOWAIT should be similar to GFP_NOWAIT? > That is allow kswapd wakeup. Yep I think that’d be fine, and probably appropriate.
diff --git a/mm/filemap.c b/mm/filemap.c index ffdfbc8b0e3c..9b747d3247c6 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -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_NOWARN; + } folio = filemap_alloc_folio(gfp, 0); if (!folio)
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 nowarn, as FGP_NOWAIT are expected to be able to handle -EAGAIN failures and retry from a suitable context without NOWAIT set. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- mm/filemap.c | 4 ++++ 1 file changed, 4 insertions(+)