public inbox for [email protected]
 help / color / mirror / Atom feed
From: Pavel Begunkov <[email protected]>
To: Jens Axboe <[email protected]>,
	Bijan Mottahedeh <[email protected]>
Cc: io-uring <[email protected]>
Subject: Re: [PATCH 1/1] io_uring: use proper references for fallback_req locking
Date: Sun, 3 May 2020 15:52:13 +0300	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

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 <[email protected]>

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);
 }


       reply	other threads:[~2020-05-03 12:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <[email protected]>
     [not found] ` <[email protected]>
2020-05-03 12:52   ` Pavel Begunkov [this message]
2020-05-04 16:12     ` [PATCH 1/1] io_uring: use proper references for fallback_req locking Jens Axboe
2020-05-04 16:28       ` Pavel Begunkov
2020-05-04 19:16       ` Bijan Mottahedeh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox