Re: [PATCH] io_uring/memmap: fix non-compound alloc fallback with memcg accounting

[Gabriel Krisman Bertazi <gabriel@krisman.be>]

David Wei <dw@davidwei.uk> writes:

> io_region_allocate_pages() first tries io_mem_alloc_compound(). If that
> fails, it uses alloc_pages_bulk_node() to allocate separate pages.
>
> The bulk allocator does not support memcg accounting. __GFP_ACCOUNT is
> unconditionally supplied, so when memcg_kmem_online() is true, it only
> allocates one page. For a multi-page request, io_uring treats this short
> result as failure and returns ENOMEM, even when the memcg has enough
> space for all pages.
>
> When this happens, fill the rest of the request with order 0 allocations
> via alloc_page(), which support memcg accounting. This maintains the
> existing fast path, while fixing the incorrect ENOMEM return when memcg
> is used.
>
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
>  io_uring/memmap.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/io_uring/memmap.c b/io_uring/memmap.c
> index cb3db32e8255..cc8ac129a0c7 100644
> --- a/io_uring/memmap.c
> +++ b/io_uring/memmap.c
> @@ -183,6 +183,7 @@ static int io_region_allocate_pages(struct io_mapped_region *mr,
>  	size_t size = io_region_size(mr);
>  	unsigned long nr_allocated;
>  	struct page **pages;
> +	struct page *page;
>  
>  	pages = kvmalloc_objs(*pages, mr->nr_pages, gfp);
>  	if (!pages)
> @@ -195,6 +196,13 @@ static int io_region_allocate_pages(struct io_mapped_region *mr,
>  
>  	nr_allocated = alloc_pages_bulk_node(gfp, NUMA_NO_NODE,
>  					     mr->nr_pages, pages);
> +	while (nr_allocated < mr->nr_pages) {
> +		page = alloc_page(gfp);
> +		if (!page)
> +			break;
> +
> +		pages[nr_allocated++] = page;
> +	}

FWIW, I looked at this recently and really think this should be the
alloc_pages_bulk_/kmem_alloc_bulk* actual semantics.  But that is a
bigger sell and I only found io_uring would actually benefit from it.

Feel free to add:

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

Perhaps also

Fixes: 1e21df691ffa ("io_uring/memmap: implement kernel allocated regions")



>  	if (nr_allocated != mr->nr_pages) {
>  		if (nr_allocated)
>  			release_pages(pages, nr_allocated);
> -- 
> 2.53.0-Meta
>

-- 
Gabriel Krisman Bertazi