* [PATCH 1/1] io_uring: use proper references for fallback_req locking @ 2020-04-30 0:47 Bijan Mottahedeh 2020-04-30 14:52 ` Jens Axboe 0 siblings, 1 reply; 6+ messages in thread From: Bijan Mottahedeh @ 2020-04-30 0:47 UTC (permalink / raw) To: axboe; +Cc: linux-block Use ctx->fallback_req address for test_and_set_bit_lock() and clear_bit_unlock(). Signed-off-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com> --- fs/io_uring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 54b7c48..8d23dc6 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1290,7 +1290,7 @@ static struct io_kiocb *io_get_fallback_req(struct io_ring_ctx *ctx) struct io_kiocb *req; req = ctx->fallback_req; - if (!test_and_set_bit_lock(0, (unsigned long *) ctx->fallback_req)) + if (!test_and_set_bit_lock(0, (unsigned long *) &ctx->fallback_req)) return req; return NULL; @@ -1377,7 +1377,7 @@ static void __io_free_req(struct io_kiocb *req) if (likely(!io_is_fallback_req(req))) kmem_cache_free(req_cachep, req); else - clear_bit_unlock(0, (unsigned long *) req->ctx->fallback_req); + clear_bit_unlock(0, (unsigned long *) &req->ctx->fallback_req); } struct req_batch { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] io_uring: use proper references for fallback_req locking 2020-04-30 0:47 [PATCH 1/1] io_uring: use proper references for fallback_req locking Bijan Mottahedeh @ 2020-04-30 14:52 ` Jens Axboe 2020-05-03 12:52 ` Pavel Begunkov 0 siblings, 1 reply; 6+ messages in thread From: Jens Axboe @ 2020-04-30 14:52 UTC (permalink / raw) To: Bijan Mottahedeh; +Cc: linux-block On 4/29/20 6:47 PM, Bijan Mottahedeh wrote: > Use ctx->fallback_req address for test_and_set_bit_lock() and > clear_bit_unlock(). Thanks, applied. -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] io_uring: use proper references for fallback_req locking 2020-04-30 14:52 ` Jens Axboe @ 2020-05-03 12:52 ` Pavel Begunkov 2020-05-04 16:12 ` Jens Axboe 0 siblings, 1 reply; 6+ messages in thread From: Pavel Begunkov @ 2020-05-03 12:52 UTC (permalink / raw) To: Jens Axboe, Bijan Mottahedeh; +Cc: io-uring On 30/04/2020 17:52, Jens Axboe wrote: > On 4/29/20 6:47 PM, Bijan Mottahedeh wrote: >> Use ctx->fallback_req address for test_and_set_bit_lock() and >> clear_bit_unlock(). > > Thanks, applied. > How about getting rid of it? As once was fairly noticed, we're screwed in many other ways in case of OOM. Otherwise we at least need to make async context allocation more resilient. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> diff --git a/fs/io_uring.c b/fs/io_uring.c index b699e9a8d247..62f9709565d0 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -282,9 +282,6 @@ struct io_ring_ctx { /* 0 is for ctx quiesce/reinit/free, 1 is for sqo_thread started */ struct completion *completions; - /* if all else fails... */ - struct io_kiocb *fallback_req; - #if defined(CONFIG_UNIX) struct socket *ring_sock; #endif @@ -911,10 +908,6 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) if (!ctx) return NULL; - ctx->fallback_req = kmem_cache_alloc(req_cachep, GFP_KERNEL); - if (!ctx->fallback_req) - goto err; - ctx->completions = kmalloc(2 * sizeof(struct completion), GFP_KERNEL); if (!ctx->completions) goto err; @@ -956,8 +949,6 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) INIT_LIST_HEAD(&ctx->inflight_list); return ctx; err: - if (ctx->fallback_req) - kmem_cache_free(req_cachep, ctx->fallback_req); kfree(ctx->completions); kfree(ctx->cancel_hash); kfree(ctx); @@ -1308,34 +1299,16 @@ static void io_cqring_add_event(struct io_kiocb *req, long res) __io_cqring_add_event(req, res, 0); } -static inline bool io_is_fallback_req(struct io_kiocb *req) -{ - return req == (struct io_kiocb *) - ((unsigned long) req->ctx->fallback_req & ~1UL); -} - -static struct io_kiocb *io_get_fallback_req(struct io_ring_ctx *ctx) -{ - struct io_kiocb *req; - - req = ctx->fallback_req; - if (!test_and_set_bit_lock(0, (unsigned long *) &ctx->fallback_req)) - return req; - - return NULL; -} - static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx, struct io_submit_state *state) { gfp_t gfp = GFP_KERNEL | __GFP_NOWARN; struct io_kiocb *req; - if (!state) { - req = kmem_cache_alloc(req_cachep, gfp); - if (unlikely(!req)) - goto fallback; - } else if (!state->free_reqs) { + if (!state) + return kmem_cache_alloc(req_cachep, gfp); + + if (!state->free_reqs) { size_t sz; int ret; @@ -1349,7 +1322,7 @@ static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx, if (unlikely(ret <= 0)) { state->reqs[0] = kmem_cache_alloc(req_cachep, gfp); if (!state->reqs[0]) - goto fallback; + return NULL; ret = 1; } state->free_reqs = ret - 1; @@ -1360,8 +1333,6 @@ static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx, } return req; -fallback: - return io_get_fallback_req(ctx); } static inline void io_put_file(struct io_kiocb *req, struct file *file, @@ -1403,10 +1374,7 @@ static void __io_free_req(struct io_kiocb *req) } percpu_ref_put(&req->ctx->refs); - if (likely(!io_is_fallback_req(req))) - kmem_cache_free(req_cachep, req); - else - clear_bit_unlock(0, (unsigned long *) &req->ctx->fallback_req); + kmem_cache_free(req_cachep, req); } struct req_batch { @@ -1699,7 +1667,7 @@ static inline unsigned int io_sqring_entries(struct io_ring_ctx *ctx) static inline bool io_req_multi_free(struct req_batch *rb, struct io_kiocb *req) { - if ((req->flags & REQ_F_LINK_HEAD) || io_is_fallback_req(req)) + if (req->flags & REQ_F_LINK_HEAD) return false; if (!(req->flags & REQ_F_FIXED_FILE) || req->io) @@ -7296,7 +7264,6 @@ static void io_ring_ctx_free(struct io_ring_ctx *ctx) put_cred(ctx->creds); kfree(ctx->completions); kfree(ctx->cancel_hash); - kmem_cache_free(req_cachep, ctx->fallback_req); kfree(ctx); } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] io_uring: use proper references for fallback_req locking 2020-05-03 12:52 ` Pavel Begunkov @ 2020-05-04 16:12 ` Jens Axboe 2020-05-04 16:28 ` Pavel Begunkov 2020-05-04 19:16 ` Bijan Mottahedeh 0 siblings, 2 replies; 6+ messages in thread From: Jens Axboe @ 2020-05-04 16:12 UTC (permalink / raw) To: Pavel Begunkov, Bijan Mottahedeh; +Cc: io-uring On 5/3/20 6:52 AM, Pavel Begunkov wrote: > On 30/04/2020 17:52, Jens Axboe wrote: >> On 4/29/20 6:47 PM, Bijan Mottahedeh wrote: >>> Use ctx->fallback_req address for test_and_set_bit_lock() and >>> clear_bit_unlock(). >> >> Thanks, applied. >> > > How about getting rid of it? As once was fairly noticed, we're screwed in many > other ways in case of OOM. Otherwise we at least need to make async context > allocation more resilient. Not sure how best to handle it, it really sucks to have things fall apart under high memory pressure, a condition that isn't that rare in production systems. But as you say, it's only a half measure currently. We could have the fallback request have req->io already allocated, though. That would provide what we need for guaranteed forward progress, even in the presence of OOM conditions. -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] io_uring: use proper references for fallback_req locking 2020-05-04 16:12 ` Jens Axboe @ 2020-05-04 16:28 ` Pavel Begunkov 2020-05-04 19:16 ` Bijan Mottahedeh 1 sibling, 0 replies; 6+ messages in thread From: Pavel Begunkov @ 2020-05-04 16:28 UTC (permalink / raw) To: Jens Axboe, Bijan Mottahedeh; +Cc: io-uring On 04/05/2020 19:12, Jens Axboe wrote: > On 5/3/20 6:52 AM, Pavel Begunkov wrote: >> On 30/04/2020 17:52, Jens Axboe wrote: >>> On 4/29/20 6:47 PM, Bijan Mottahedeh wrote: >>>> Use ctx->fallback_req address for test_and_set_bit_lock() and >>>> clear_bit_unlock(). >>> >>> Thanks, applied. >>> >> >> How about getting rid of it? As once was fairly noticed, we're screwed in many >> other ways in case of OOM. Otherwise we at least need to make async context >> allocation more resilient. > > Not sure how best to handle it, it really sucks to have things fall apart > under high memory pressure, a condition that isn't that rare in production > systems. But as you say, it's only a half measure currently. We could have > the fallback request have req->io already allocated, though. That would > provide what we need for guaranteed forward progress, even in the presence > of OOM conditions. Good idea. +extend it to work with links as a next step. E.g. for short links (2-3 reqs). -- Pavel Begunkov ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] io_uring: use proper references for fallback_req locking 2020-05-04 16:12 ` Jens Axboe 2020-05-04 16:28 ` Pavel Begunkov @ 2020-05-04 19:16 ` Bijan Mottahedeh 1 sibling, 0 replies; 6+ messages in thread From: Bijan Mottahedeh @ 2020-05-04 19:16 UTC (permalink / raw) To: Jens Axboe, Pavel Begunkov; +Cc: io-uring On 5/4/2020 9:12 AM, Jens Axboe wrote: > On 5/3/20 6:52 AM, Pavel Begunkov wrote: >> On 30/04/2020 17:52, Jens Axboe wrote: >>> On 4/29/20 6:47 PM, Bijan Mottahedeh wrote: >>>> Use ctx->fallback_req address for test_and_set_bit_lock() and >>>> clear_bit_unlock(). >>> Thanks, applied. >>> >> How about getting rid of it? As once was fairly noticed, we're screwed in many >> other ways in case of OOM. Otherwise we at least need to make async context >> allocation more resilient. > Not sure how best to handle it, it really sucks to have things fall apart > under high memory pressure, a condition that isn't that rare in production > systems. But as you say, it's only a half measure currently. We could have > the fallback request have req->io already allocated, though. That would > provide what we need for guaranteed forward progress, even in the presence > of OOM conditions. > A somewhat related question, would it make sense to have (configurable) pre-allocated requests, to be used first if low latency is a priority for a ring, or would the allocation overhead be negligible compared to the actual I/O? This would be the flip side of fallback in a sense. Thanks. --bijan --bijan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-04 19:16 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-30 0:47 [PATCH 1/1] io_uring: use proper references for fallback_req locking Bijan Mottahedeh 2020-04-30 14:52 ` Jens Axboe 2020-05-03 12:52 ` Pavel Begunkov 2020-05-04 16:12 ` Jens Axboe 2020-05-04 16:28 ` Pavel Begunkov 2020-05-04 19:16 ` Bijan Mottahedeh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.