[PATCH v2] mm: honor FGP_NOWAIT for page cache page allocation

[PATCH v2] mm: honor FGP_NOWAIT for page cache page allocation

[Jens Axboe]

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.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---

V2:
- Improve commit message
- Add GFP_NOWAIT to allow kswapd wakeup

 mm/filemap.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/filemap.c b/mm/filemap.c
index ffdfbc8b0e3c..254931a6e3ed 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_NOWAIT | __GFP_NOWARN;
+		}
 
 		folio = filemap_alloc_folio(gfp, 0);
 		if (!folio)
-- 
2.35.1

-- 
Jens Axboe

Re: [PATCH v2] mm: honor FGP_NOWAIT for page cache page allocation

[Shakeel Butt]

On Sat, Jul 02, 2022 at 09:43:32AM -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 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.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>

Reviewed-by: Shakeel Butt <shakeelb@google.com>

Re: [PATCH v2] mm: honor FGP_NOWAIT for page cache page allocation

[Matthew Wilcox]

On Sat, Jul 02, 2022 at 09:43:32AM -0600, Jens Axboe wrote:
> +++ 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_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)

Re: [PATCH v2] mm: honor FGP_NOWAIT for page cache page allocation

[Jens Axboe]

On 7/3/22 2:37 AM, Matthew Wilcox wrote:
> On Sat, Jul 02, 2022 at 09:43:32AM -0600, Jens Axboe wrote:
>> +++ 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_NOWAIT | __GFP_NOWARN;
>> +		}
> 
> Wouldn't it be simpler to write:
> 
> 		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.

> (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...

-- 
Jens Axboe