* [PATCH 0/3] poll fixes
@ 2021-09-22 10:12 Hao Xu
2021-09-22 10:12 ` [PATCH 1/3] io_uring: fix race between poll completion and cancel_hash insertion Hao Xu
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Hao Xu @ 2021-09-22 10:12 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
Three poll fixes.
Hao Xu (3):
io_uring: fix race between poll completion and cancel_hash insertion
io_uring: fix lacking of EPOLLONESHOT
io_uring: fix potential req refcount underflow
fs/io_uring.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
--
2.24.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] io_uring: fix race between poll completion and cancel_hash insertion
2021-09-22 10:12 [PATCH 0/3] poll fixes Hao Xu
@ 2021-09-22 10:12 ` Hao Xu
2021-09-22 10:12 ` [PATCH 2/3] io_uring: fix lacking of EPOLLONESHOT Hao Xu
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Hao Xu @ 2021-09-22 10:12 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
If poll arming and poll completion runs parallelly, there maybe races.
For instance, run io_poll_add in iowq and io_poll_task_func in original
context, then:
iowq original context
io_poll_add
vfs_poll
(interruption happens
tw queued to original
context) io_poll_task_func
generate cqe
del from cancel_hash[]
if !poll.done
insert to cancel_hash[]
The entry left in cancel_hash[], similar case for fast poll.
Fix it by set poll.done = true when del from cancel_hash[].
Fixes: 5082620fb2ca ("io_uring: terminate multishot poll for CQ ring overflow")
Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
fs/io_uring.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 91e4c89abf78..4b0a40ad28b0 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5340,10 +5340,8 @@ static bool __io_poll_complete(struct io_kiocb *req, __poll_t mask)
}
if (req->poll.events & EPOLLONESHOT)
flags = 0;
- if (!io_cqring_fill_event(ctx, req->user_data, error, flags)) {
- req->poll.done = true;
+ if (!io_cqring_fill_event(ctx, req->user_data, error, flags))
flags = 0;
- }
if (flags & IORING_CQE_F_MORE)
ctx->cq_extra++;
@@ -5374,6 +5372,7 @@ static void io_poll_task_func(struct io_kiocb *req, bool *locked)
if (done) {
io_poll_remove_double(req);
hash_del(&req->hash_node);
+ req->poll.done = true;
} else {
req->result = 0;
add_wait_queue(req->poll.head, &req->poll.wait);
@@ -5511,6 +5510,7 @@ static void io_async_task_func(struct io_kiocb *req, bool *locked)
hash_del(&req->hash_node);
io_poll_remove_double(req);
+ apoll->poll.done = true;
spin_unlock(&ctx->completion_lock);
if (!READ_ONCE(apoll->poll.canceled))
--
2.24.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] io_uring: fix lacking of EPOLLONESHOT
2021-09-22 10:12 [PATCH 0/3] poll fixes Hao Xu
2021-09-22 10:12 ` [PATCH 1/3] io_uring: fix race between poll completion and cancel_hash insertion Hao Xu
@ 2021-09-22 10:12 ` Hao Xu
2021-09-22 10:12 ` [PATCH 3/3] io_uring: fix potential req refcount underflow Hao Xu
2021-09-22 15:55 ` [PATCH 0/3] poll fixes Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Hao Xu @ 2021-09-22 10:12 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
We should set EPOLLONESHOT if cqring_fill_event() returns false since
io_poll_add() decides to put req or not by it.
Fixes: 5082620fb2ca ("io_uring: terminate multishot poll for CQ ring overflow")
Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
fs/io_uring.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 4b0a40ad28b0..b7c9fcce2de2 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5340,8 +5340,10 @@ static bool __io_poll_complete(struct io_kiocb *req, __poll_t mask)
}
if (req->poll.events & EPOLLONESHOT)
flags = 0;
- if (!io_cqring_fill_event(ctx, req->user_data, error, flags))
+ if (!io_cqring_fill_event(ctx, req->user_data, error, flags)) {
+ req->poll.events |= EPOLLONESHOT;
flags = 0;
+ }
if (flags & IORING_CQE_F_MORE)
ctx->cq_extra++;
--
2.24.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] io_uring: fix potential req refcount underflow
2021-09-22 10:12 [PATCH 0/3] poll fixes Hao Xu
2021-09-22 10:12 ` [PATCH 1/3] io_uring: fix race between poll completion and cancel_hash insertion Hao Xu
2021-09-22 10:12 ` [PATCH 2/3] io_uring: fix lacking of EPOLLONESHOT Hao Xu
@ 2021-09-22 10:12 ` Hao Xu
2021-09-22 15:55 ` [PATCH 0/3] poll fixes Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Hao Xu @ 2021-09-22 10:12 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi
For multishot mode, there may be cases like:
iowq original context
io_poll_add
_arm_poll()
mask = vfs_poll() is not 0
if mask
(2) io_poll_complete()
compl_unlock
(interruption happens
tw queued to original
context)
io_poll_task_func()
compl_lock
(3) done = io_poll_complete() is true
compl_unlock
put req ref
(1) if (poll->flags & EPOLLONESHOT)
put req ref
EPOLLONESHOT flag in (1) may be from (2) or (3), so there are multiple
combinations that can cause ref underfow.
Let's address it by:
- check the return value in (2) as done
- change (1) to if (done)
in this way, we only do ref put in (1) if 'oneshot flag' is from
(2)
- do poll.done check in io_poll_task_func(), so that we won't put ref
for the second time.
Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
fs/io_uring.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index b7c9fcce2de2..b008edccad46 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5370,6 +5370,10 @@ static void io_poll_task_func(struct io_kiocb *req, bool *locked)
} else {
bool done;
+ if (req->poll.done) {
+ spin_unlock(&ctx->completion_lock);
+ return;
+ }
done = __io_poll_complete(req, req->result);
if (done) {
io_poll_remove_double(req);
@@ -5833,6 +5837,7 @@ static int io_poll_add(struct io_kiocb *req, unsigned int issue_flags)
struct io_ring_ctx *ctx = req->ctx;
struct io_poll_table ipt;
__poll_t mask;
+ bool done;
ipt.pt._qproc = io_poll_queue_proc;
@@ -5841,13 +5846,13 @@ static int io_poll_add(struct io_kiocb *req, unsigned int issue_flags)
if (mask) { /* no async, we'd stolen it */
ipt.error = 0;
- io_poll_complete(req, mask);
+ done = io_poll_complete(req, mask);
}
spin_unlock(&ctx->completion_lock);
if (mask) {
io_cqring_ev_posted(ctx);
- if (poll->events & EPOLLONESHOT)
+ if (done)
io_put_req(req);
}
return ipt.error;
--
2.24.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] poll fixes
2021-09-22 10:12 [PATCH 0/3] poll fixes Hao Xu
` (2 preceding siblings ...)
2021-09-22 10:12 ` [PATCH 3/3] io_uring: fix potential req refcount underflow Hao Xu
@ 2021-09-22 15:55 ` Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2021-09-22 15:55 UTC (permalink / raw)
To: Hao Xu; +Cc: io-uring, Pavel Begunkov, Joseph Qi
On 9/22/21 4:12 AM, Hao Xu wrote:
> Three poll fixes.
>
> Hao Xu (3):
> io_uring: fix race between poll completion and cancel_hash insertion
> io_uring: fix lacking of EPOLLONESHOT
> io_uring: fix potential req refcount underflow
>
> fs/io_uring.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
These look good to me, nice fixes! I'll run this through some testing and
get them applied for 5.15. Thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-22 15:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 10:12 [PATCH 0/3] poll fixes Hao Xu
2021-09-22 10:12 ` [PATCH 1/3] io_uring: fix race between poll completion and cancel_hash insertion Hao Xu
2021-09-22 10:12 ` [PATCH 2/3] io_uring: fix lacking of EPOLLONESHOT Hao Xu
2021-09-22 10:12 ` [PATCH 3/3] io_uring: fix potential req refcount underflow Hao Xu
2021-09-22 15:55 ` [PATCH 0/3] poll fixes Jens Axboe
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.