Re: [PATCH v3 1/2] io_uring: cmpxchg for poll arm refs release

[Pavel Begunkov <asml.silence@gmail.com>]

On 11/20/22 16:57, Pavel Begunkov wrote:
> Replace atomically substracting the ownership reference at the end of
> arming a poll with a cmpxchg. We try to release ownership by setting 0
> assuming that poll_refs didn't change while we were arming. If it did
> change, we keep the ownership and use it to queue a tw, which is fully
> capable to process all events and (even tolerates spurious wake ups).
> 
> It's a bit more elegant as we reduce races b/w setting the cancellation
> flag and getting refs with this release, and with that we don't have to
> worry about any kinds of underflows. It's not the fastest path for
> polling. The performance difference b/w cmpxchg and atomic dec is
> usually negligible and it's not the fastest path.

Jens, can you add a couple of tags? Not a fix but the second patch
depends on it but applies cleanly even without 1/2, which may mess
things up.

Cc: stable@vger.kernel.org
Fixes: aa43477b04025 ("io_uring: poll rework")


> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   io_uring/poll.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/io_uring/poll.c b/io_uring/poll.c
> index 055632e9092a..1b78b527075d 100644
> --- a/io_uring/poll.c
> +++ b/io_uring/poll.c
> @@ -518,7 +518,6 @@ static int __io_arm_poll_handler(struct io_kiocb *req,
>   				 unsigned issue_flags)
>   {
>   	struct io_ring_ctx *ctx = req->ctx;
> -	int v;
>   
>   	INIT_HLIST_NODE(&req->hash_node);
>   	req->work.cancel_seq = atomic_read(&ctx->cancel_seq);
> @@ -586,11 +585,10 @@ static int __io_arm_poll_handler(struct io_kiocb *req,
>   
>   	if (ipt->owning) {
>   		/*
> -		 * Release ownership. If someone tried to queue a tw while it was
> -		 * locked, kick it off for them.
> +		 * Try to release ownership. If we see a change of state, e.g.
> +		 * poll was waken up, queue up a tw, it'll deal with it.
>   		 */
> -		v = atomic_dec_return(&req->poll_refs);
> -		if (unlikely(v & IO_POLL_REF_MASK))
> +		if (atomic_cmpxchg(&req->poll_refs, 1, 0) != 1)
>   			__io_poll_execute(req, 0);
>   	}
>   	return 0;

-- 
Pavel Begunkov