Commit de08356f authored by Pavel Begunkov's avatar Pavel Begunkov Committed by Jens Axboe
Browse files

io_uring: refactor poll arm error handling 



__io_arm_poll_handler() errors parsing is a horror, in case it failed it
returns 0 and the caller is expected to look at ipt.error, which already
led us to a number of problems before.

When it returns a valid mask, leave it as it's not, i.e. return 1 and
store the mask in ipt.result_mask. In case of a failure that can be
handled inline return an error code (negative value), and return 0 if
__io_arm_poll_handler() took ownership of the request and will complete
it.

Signed-off-by: default avatarPavel Begunkov <asml.silence@gmail.com>
Link: https://lore.kernel.org/r/018cacdaef5fe95d7dc56b32e85d752cab7607f6.1655990418.git.asml.silence@gmail.com


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 063a0079

io_uring/poll.c

+21 −23
Original line number Diff line number Diff line
@@ -435,6 +435,12 @@ static void io_poll_queue_proc(struct file *file, struct wait_queue_head *head, 
			(struct io_poll **) &pt->req->async_data);

}



/*

 * Returns 0 when it's handed over for polling. The caller owns the requests if

 * it returns non-zero, but otherwise should not touch it. Negative values

 * contain an error code. When the result is >0, the polling has completed

 * inline and ipt.result_mask is set to the mask.

 */

static int __io_arm_poll_handler(struct io_kiocb *req,

				 struct io_poll *poll,

				 struct io_poll_table *ipt, __poll_t mask)
@@ -461,19 +467,23 @@ static int __io_arm_poll_handler(struct io_kiocb *req, 
	atomic_set(&req->poll_refs, 1);

	mask = vfs_poll(req->file, &ipt->pt) & poll->events;



	if (mask &&

	   ((poll->events & (EPOLLET|EPOLLONESHOT)) == (EPOLLET|EPOLLONESHOT))) {

	if (unlikely(ipt->error || !ipt->nr_entries)) {

		io_poll_remove_entries(req);



		if (mask && (poll->events & EPOLLET)) {

			ipt->result_mask = mask;

		/* no one else has access to the req, forget about the ref */

			return 1;

		} else {

			return ipt->error ?: -EINVAL;

		}

	}



	if (!mask && unlikely(ipt->error || !ipt->nr_entries)) {

	if (mask &&

	   ((poll->events & (EPOLLET|EPOLLONESHOT)) == (EPOLLET|EPOLLONESHOT))) {

		io_poll_remove_entries(req);

		if (!ipt->error)

			ipt->error = -EINVAL;

		return 0;

		ipt->result_mask = mask;

		/* no one else has access to the req, forget about the ref */

		return 1;

	}



	if (req->flags & REQ_F_HASH_LOCKED)
@@ -482,12 +492,6 @@ static int __io_arm_poll_handler(struct io_kiocb *req, 
		io_poll_req_insert(req);



	if (mask && (poll->events & EPOLLET)) {

		/* can't multishot if failed, just queue the event we've got */

		if (unlikely(ipt->error || !ipt->nr_entries)) {

			poll->events |= EPOLLONESHOT;

			req->apoll_events |= EPOLLONESHOT;

			ipt->error = 0;

		}

		__io_poll_execute(req, mask);

		return 0;

	}
@@ -582,9 +586,8 @@ int io_arm_poll_handler(struct io_kiocb *req, unsigned issue_flags) 
	io_kbuf_recycle(req, issue_flags);



	ret = __io_arm_poll_handler(req, &apoll->poll, &ipt, mask);

	if (ret || ipt.error)

		return ret ? IO_APOLL_READY : IO_APOLL_ABORTED;



	if (ret)

		return ret > 0 ? IO_APOLL_READY : IO_APOLL_ABORTED;

	trace_io_uring_poll_arm(req, mask, apoll->poll.events);

	return IO_APOLL_OK;

}
@@ -815,16 +818,11 @@ int io_poll_add(struct io_kiocb *req, unsigned int issue_flags) 
		req->flags &= ~REQ_F_HASH_LOCKED;



	ret = __io_arm_poll_handler(req, poll, &ipt, poll->events);

	if (ret) {

	if (ret > 0) {

		io_req_set_res(req, ipt.result_mask, 0);

		return IOU_OK;

	}

	if (ipt.error) {

		req_set_fail(req);

		return ipt.error;

	}



	return IOU_ISSUE_SKIP_COMPLETE;

	return ret ?: IOU_ISSUE_SKIP_COMPLETE;

}



int io_poll_remove(struct io_kiocb *req, unsigned int issue_flags)