diff mbox series

aio: Support read/write with non-iter file-ops

Message ID 20190718231054.8175-1-bjorn.andersson@linaro.org
State New
Headers show
Series aio: Support read/write with non-iter file-ops | expand

Commit Message

Bjorn Andersson July 18, 2019, 11:10 p.m. UTC 
Implement a wrapper for aio_read()/write() to allow async IO on files
not implementing the iter version of read/write, such as sysfs. This
mimics how readv/writev uses non-iter ops in do_loop_readv_writev().

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

---
 fs/aio.c | 52 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

-- 
2.18.0

Comments

Al Viro July 18, 2019, 11:17 p.m. UTC | #1 
On Thu, Jul 18, 2019 at 04:10:54PM -0700, Bjorn Andersson wrote:
> Implement a wrapper for aio_read()/write() to allow async IO on files

> not implementing the iter version of read/write, such as sysfs. This

> mimics how readv/writev uses non-iter ops in do_loop_readv_writev().


IDGI.  How would that IO manage to be async?  And what's the point
using aio in such situations in the first place?
Bjorn Andersson July 18, 2019, 11:43 p.m. UTC | #2 
On Thu 18 Jul 16:17 PDT 2019, Al Viro wrote:

> On Thu, Jul 18, 2019 at 04:10:54PM -0700, Bjorn Andersson wrote:

> > Implement a wrapper for aio_read()/write() to allow async IO on files

> > not implementing the iter version of read/write, such as sysfs. This

> > mimics how readv/writev uses non-iter ops in do_loop_readv_writev().

> 

> IDGI.  How would that IO manage to be async?  And what's the point

> using aio in such situations in the first place?


The point is that an application using aio to submit io operations on a
set of files, can use the same mechanism to read/write files that
happens to be implemented by driver only implementing read/write (not
read_iter/write_iter) in the registered file_operations struct, such as
kernfs.

In this particular case I have a sysfs file that is accessing hardware
and hence will block for a while and using this patch I can io_submit()
a write and handle the completion of this in my normal event loop.


Each individual io operation will be just as synchronous as the current
iter-based mechanism - for the drivers that implement that.

Regards,
Bjorn
Benjamin LaHaise July 18, 2019, 11:56 p.m. UTC | #3 
On Thu, Jul 18, 2019 at 04:43:52PM -0700, Bjorn Andersson wrote:
> On Thu 18 Jul 16:17 PDT 2019, Al Viro wrote:

> 

> > On Thu, Jul 18, 2019 at 04:10:54PM -0700, Bjorn Andersson wrote:

> > > Implement a wrapper for aio_read()/write() to allow async IO on files

> > > not implementing the iter version of read/write, such as sysfs. This

> > > mimics how readv/writev uses non-iter ops in do_loop_readv_writev().

> > 

> > IDGI.  How would that IO manage to be async?  And what's the point

> > using aio in such situations in the first place?

> 

> The point is that an application using aio to submit io operations on a

> set of files, can use the same mechanism to read/write files that

> happens to be implemented by driver only implementing read/write (not

> read_iter/write_iter) in the registered file_operations struct, such as

> kernfs.

> 

> In this particular case I have a sysfs file that is accessing hardware

> and hence will block for a while and using this patch I can io_submit()

> a write and handle the completion of this in my normal event loop.

> 

> 

> Each individual io operation will be just as synchronous as the current

> iter-based mechanism - for the drivers that implement that.


Just adding the fops is not enough.  I have patches floating around at
Solace that add thread based fallbacks for files that don't have an aio
read / write implementation, but I'm not working on that code any more.
The thread based methods were quite useful in applications that had a need
for using other kernel infrastructure in their main event loops.

		-ben

> Regards,

> Bjorn

> 


-- 
"Thought is the essence of where you are now."
Al Viro July 19, 2019, 12:07 a.m. UTC | #4 
On Thu, Jul 18, 2019 at 04:43:52PM -0700, Bjorn Andersson wrote:
> On Thu 18 Jul 16:17 PDT 2019, Al Viro wrote:

> 

> > On Thu, Jul 18, 2019 at 04:10:54PM -0700, Bjorn Andersson wrote:

> > > Implement a wrapper for aio_read()/write() to allow async IO on files

> > > not implementing the iter version of read/write, such as sysfs. This

> > > mimics how readv/writev uses non-iter ops in do_loop_readv_writev().

> > 

> > IDGI.  How would that IO manage to be async?  And what's the point

> > using aio in such situations in the first place?

> 

> The point is that an application using aio to submit io operations on a

> set of files,


... for no reason whatsoever, I take it?

> can use the same mechanism to read/write files that

> happens to be implemented by driver only implementing read/write (not

> read_iter/write_iter) in the registered file_operations struct, such as

> kernfs.


... except that it still has to support the kernels that don't have
your patch, so the fallback in userland is *not* going away.
Bjorn Andersson July 19, 2019, 12:36 a.m. UTC | #5 
On Thu 18 Jul 16:56 PDT 2019, Benjamin LaHaise wrote:

> On Thu, Jul 18, 2019 at 04:43:52PM -0700, Bjorn Andersson wrote:

> > On Thu 18 Jul 16:17 PDT 2019, Al Viro wrote:

> > 

> > > On Thu, Jul 18, 2019 at 04:10:54PM -0700, Bjorn Andersson wrote:

> > > > Implement a wrapper for aio_read()/write() to allow async IO on files

> > > > not implementing the iter version of read/write, such as sysfs. This

> > > > mimics how readv/writev uses non-iter ops in do_loop_readv_writev().

> > > 

> > > IDGI.  How would that IO manage to be async?  And what's the point

> > > using aio in such situations in the first place?

> > 

> > The point is that an application using aio to submit io operations on a

> > set of files, can use the same mechanism to read/write files that

> > happens to be implemented by driver only implementing read/write (not

> > read_iter/write_iter) in the registered file_operations struct, such as

> > kernfs.

> > 

> > In this particular case I have a sysfs file that is accessing hardware

> > and hence will block for a while and using this patch I can io_submit()

> > a write and handle the completion of this in my normal event loop.

> > 

> > 

> > Each individual io operation will be just as synchronous as the current

> > iter-based mechanism - for the drivers that implement that.

> 

> Just adding the fops is not enough.  I have patches floating around at

> Solace that add thread based fallbacks for files that don't have an aio

> read / write implementation, but I'm not working on that code any more.


My bad. Took another look and now I see the bigger picture of how this
is currently implemented and why just adding the fops would defeat the
purpose of the api.

Sorry for the noise.

> The thread based methods were quite useful in applications that had a need

> for using other kernel infrastructure in their main event loops.

> 


Yes indeed.

Regards,
Bjorn
diff mbox series

Patch

diff --git a/fs/aio.c b/fs/aio.c
index f9f441b59966..0137a1a9bef1 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1514,12 +1514,44 @@  static inline void aio_rw_done(struct kiocb *req, ssize_t ret)
 	}
 }
 
+static ssize_t aio_iter_readv_writev(struct file *file, struct kiocb *req,
+				     struct iov_iter *iter, int type)
+{
+	ssize_t ret = 0;
+	ssize_t nr;
+
+	while (iov_iter_count(iter)) {
+		struct iovec iovec = iov_iter_iovec(iter);
+
+		if (type == READ) {
+			nr = file->f_op->read(file, iovec.iov_base,
+					      iovec.iov_len, &req->ki_pos);
+		} else {
+			nr = file->f_op->write(file, iovec.iov_base,
+					       iovec.iov_len, &req->ki_pos);
+		}
+
+		if (nr < 0) {
+			ret = nr;
+			break;
+		}
+
+		ret += nr;
+		if (nr != iovec.iov_len)
+			break;
+		iov_iter_advance(iter, nr);
+	}
+
+	return ret;
+}
+
 static int aio_read(struct kiocb *req, const struct iocb *iocb,
 			bool vectored, bool compat)
 {
 	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
 	struct iov_iter iter;
 	struct file *file;
+	ssize_t count;
 	int ret;
 
 	ret = aio_prep_rw(req, iocb);
@@ -1529,15 +1561,18 @@  static int aio_read(struct kiocb *req, const struct iocb *iocb,
 	if (unlikely(!(file->f_mode & FMODE_READ)))
 		return -EBADF;
 	ret = -EINVAL;
-	if (unlikely(!file->f_op->read_iter))
-		return -EINVAL;
 
 	ret = aio_setup_rw(READ, iocb, &iovec, vectored, compat, &iter);
 	if (ret < 0)
 		return ret;
 	ret = rw_verify_area(READ, file, &req->ki_pos, iov_iter_count(&iter));
-	if (!ret)
-		aio_rw_done(req, call_read_iter(file, req, &iter));
+	if (!ret) {
+		if (likely(file->f_op->read_iter))
+			count = call_read_iter(file, req, &iter);
+		else
+			count = aio_iter_readv_writev(file, req, &iter, READ);
+		aio_rw_done(req, count);
+	}
 	kfree(iovec);
 	return ret;
 }
@@ -1548,6 +1583,7 @@  static int aio_write(struct kiocb *req, const struct iocb *iocb,
 	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
 	struct iov_iter iter;
 	struct file *file;
+	ssize_t count;
 	int ret;
 
 	ret = aio_prep_rw(req, iocb);
@@ -1557,8 +1593,6 @@  static int aio_write(struct kiocb *req, const struct iocb *iocb,
 
 	if (unlikely(!(file->f_mode & FMODE_WRITE)))
 		return -EBADF;
-	if (unlikely(!file->f_op->write_iter))
-		return -EINVAL;
 
 	ret = aio_setup_rw(WRITE, iocb, &iovec, vectored, compat, &iter);
 	if (ret < 0)
@@ -1577,7 +1611,11 @@  static int aio_write(struct kiocb *req, const struct iocb *iocb,
 			__sb_writers_release(file_inode(file)->i_sb, SB_FREEZE_WRITE);
 		}
 		req->ki_flags |= IOCB_WRITE;
-		aio_rw_done(req, call_write_iter(file, req, &iter));
+		if (likely(file->f_op->write_iter))
+			count = call_write_iter(file, req, &iter);
+		else
+			count = aio_iter_readv_writev(file, req, &iter, WRITE);
+		aio_rw_done(req, count);
 	}
 	kfree(iovec);
 	return ret;