Re: [PATCH] io_uring: Use slab for struct io_buffer objects

[Gabriel Krisman Bertazi <krisman@suse.de>]

Jeff Moyer <jmoyer@redhat.com> writes:

> Hi, Gabriel,
>
> I just have a couple of comments.  I don't have an opinion on whether it
> makes sense to replace the existing allocator.
>
> -Jeff
>
>> @@ -362,11 +363,12 @@ int io_provide_buffers_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
>>  	return 0;
>>  }
>>  
>> +#define IO_BUFFER_ALLOC_BATCH (PAGE_SIZE/sizeof(struct io_buffer))
>> +
>>  static int io_refill_buffer_cache(struct io_ring_ctx *ctx)
>>  {
>> -	struct io_buffer *buf;
>> -	struct page *page;
>> -	int bufs_in_page;
>> +	struct io_buffer *bufs[IO_BUFFER_ALLOC_BATCH];
>
> That's a pretty large on-stack allocation.

Indeed, that is definitely too large. Thanks for pointing it out.

Also,  I just noticed the define above should actually read:

#define IO_BUFFER_ALLOC_BATCH (PAGE_SIZE/sizeof(struct io_buffer *))

I'll follow up with a v2, after I retest the impact a smaller allocation
batch would have.

>
>> +	allocated = kmem_cache_alloc_bulk(io_buf_cachep, GFP_KERNEL_ACCOUNT,
>> +					  ARRAY_SIZE(bufs), (void **) bufs);
>> +	if (unlikely(allocated <= 0)) {
>
> Can't be less than 0.

Thanks, will fix.

>

-- 
Gabriel Krisman Bertazi