Jeff Moyer <jmoyer@xxxxxxxxxx> 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