Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: support fds that have no {read,write}_iter support #12

Closed
CarterLi opened this issue Sep 23, 2019 · 4 comments
Closed

Comments

@CarterLi
Copy link
Contributor

There is no way to know whether a file descriptor support {read,write}_iter, and sometimes we don't even know the type of fds, for example STDIN/OUT/ERR_FILENO. We have to try IORING_OP_PREADV first. If we get -EINVAL, we have to fall back to IORING_OP_POLL_ADD and plain read, which is inconvenient and slow.

Ref: libuv/libuv#2322 (comment)

@CarterLi CarterLi changed the title Feature request: support for fds that have no {read,write}_iter support Feature request: support fds that have no {read,write}_iter support Sep 23, 2019
@axboe
Copy link
Owner

axboe commented Sep 23, 2019

Agree, and it should not be hard, only issue is that we always have to punt to async for handling non iter based operations. Five min hack, haven't tested it yet:

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9d8e703bc851..0454565d501e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1298,6 +1298,42 @@ static void io_async_list_note(int rw, struct io_kiocb *req, size_t len)
 	}
 }
 
+static ssize_t loop_rw_iter(int type, struct file *file, struct kiocb *kiocb,
+			   struct iov_iter *iter)
+{
+	ssize_t ret = 0;
+
+	if (kiocb->ki_flags & IOCB_HIPRI)
+		return -EOPNOTSUPP;
+	if (kiocb->ki_flags & IOCB_NOWAIT)
+		return -EAGAIN;
+
+	while (iov_iter_count(iter)) {
+		struct iovec iovec = iov_iter_iovec(iter);
+		ssize_t nr;
+
+		if (type == READ) {
+			nr = file->f_op->read(file, iovec.iov_base,
+					      iovec.iov_len, &kiocb->ki_pos);
+		} else {
+			nr = file->f_op->write(file, iovec.iov_base,
+					       iovec.iov_len, &kiocb->ki_pos);
+		}
+
+		if (nr < 0) {
+			if (!ret)
+				ret = nr;
+			break;
+		}
+		ret += nr;
+		if (nr != iovec.iov_len)
+			break;
+		iov_iter_advance(iter, nr);
+	}
+
+	return ret;
+}
+
 static int io_read(struct io_kiocb *req, const struct sqe_submit *s,
 		   bool force_nonblock)
 {
@@ -1315,8 +1351,6 @@ static int io_read(struct io_kiocb *req, const struct sqe_submit *s,
 
 	if (unlikely(!(file->f_mode & FMODE_READ)))
 		return -EBADF;
-	if (unlikely(!file->f_op->read_iter))
-		return -EINVAL;
 
 	ret = io_import_iovec(req->ctx, READ, s, &iovec, &iter);
 	if (ret < 0)
@@ -1331,7 +1365,11 @@ static int io_read(struct io_kiocb *req, const struct sqe_submit *s,
 	if (!ret) {
 		ssize_t ret2;
 
-		ret2 = call_read_iter(file, kiocb, &iter);
+		if (file->f_op->read_iter)
+			ret2 = call_read_iter(file, kiocb, &iter);
+		else
+			ret2 = loop_rw_iter(READ, file, kiocb, &iter);
+
 		/*
 		 * In case of a short read, punt to async. This can happen
 		 * if we have data partially cached. Alternatively we can
@@ -1376,8 +1414,6 @@ static int io_write(struct io_kiocb *req, const struct sqe_submit *s,
 	file = kiocb->ki_filp;
 	if (unlikely(!(file->f_mode & FMODE_WRITE)))
 		return -EBADF;
-	if (unlikely(!file->f_op->write_iter))
-		return -EINVAL;
 
 	ret = io_import_iovec(req->ctx, WRITE, s, &iovec, &iter);
 	if (ret < 0)
@@ -1415,7 +1451,10 @@ static int io_write(struct io_kiocb *req, const struct sqe_submit *s,
 		}
 		kiocb->ki_flags |= IOCB_WRITE;
 
-		ret2 = call_write_iter(file, kiocb, &iter);
+		if (file->f_op->write_iter)
+			ret2 = call_write_iter(file, kiocb, &iter);
+		else
+			ret2 = loop_rw_iter(WRITE, file, kiocb, &iter);
 		if (!force_nonblock || ret2 != -EAGAIN) {
 			io_rw_done(kiocb, ret2);
 		} else {

@axboe
Copy link
Owner

axboe commented Sep 23, 2019

Posted it here:

https://lore.kernel.org/linux-block/6e9531fe-9047-7005-4625-652490419cc3@kernel.dk/

and here's a test case as well:

http://git.kernel.dk/cgit/liburing/commit/?id=556960942eaa69fd53544932f00db3fa9f196e00

If you get a chance to test it, please reply back to the original email above with an acknowledgement.

@CarterLi
Copy link
Contributor Author

That was GREAT! io_uring is getting more feature complete then ever! Thanks for you quick patch!

But sorry, I have no idea how to build the kernel, and have to wait for a distro.

@axboe
Copy link
Owner

axboe commented Sep 23, 2019

Thanks for prodding me about these issues! Don't worry about not being able to test it, I ran it through some testing here, seems good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants