[PATCH 7/9] aio: add aio_kernel_() interface

[PATCH 7/9] aio: add aio_kernel_() interface

[Ming Lei]

From: Dave Kleikamp <dave.kleikamp@oracle.com>

This adds an interface that lets kernel callers submit aio iocbs without
going through the user space syscalls.  This lets kernel callers avoid
the management limits and overhead of the context.  It will also let us
integrate aio operations with other kernel apis that the user space
interface doesn't have access to.

This patch is based on Dave's posts in below links:

	https://lkml.org/lkml/2013/10/16/365
	https://groups.google.com/forum/#!topic/linux.kernel/l7mogGJZoKQ

And most of the patch is from Dave's directly.

Cc: Zach Brown <zab@zabbo.net>
Cc: Dave Kleikamp <dave.kleikamp@oracle.com>
Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-aio@kvack.org (open list:AIO)
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 fs/aio.c            |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/aio.h |   21 +++++++++-
 2 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/fs/aio.c b/fs/aio.c
index d93bfa6..7a081b7 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -948,6 +948,9 @@ void aio_complete(struct kiocb *iocb, long res, long res2)
 		iocb->ki_ctx = ERR_PTR(-EXDEV);
 		wake_up_process(iocb->ki_obj.tsk);
 		return;
+	} else if (is_kernel_kiocb(iocb)) {
+		iocb->ki_obj.complete(iocb->ki_user_data, res);
+		return;
 	}
 
 	if (iocb->ki_list.next) {
@@ -1395,6 +1398,117 @@ rw_common:
 	return 0;
 }
 
+/*
+ * This allocates an iocb that will be used to submit and track completion of
+ * an IO that is issued from kernel space.
+ *
+ * The caller is expected to call the appropriate aio_kernel_init_() functions
+ * and then call aio_kernel_submit().  From that point forward progress is
+ * guaranteed by the file system aio method.  Eventually the caller's
+ * completion callback will be called.
+ *
+ * These iocbs are special.  They don't have a context, we don't limit the
+ * number pending, and they can't be canceled.
+ */
+struct kiocb *aio_kernel_alloc(gfp_t gfp, unsigned extra)
+{
+	return kzalloc(sizeof(struct kiocb) + extra, gfp);
+}
+EXPORT_SYMBOL_GPL(aio_kernel_alloc);
+
+void aio_kernel_free(struct kiocb *iocb)
+{
+	kfree(iocb);
+}
+EXPORT_SYMBOL_GPL(aio_kernel_free);
+
+/*
+ * ptr and count can be a buff and bytes or an iov and segs.
+ */
+void aio_kernel_init_rw(struct kiocb *iocb, struct file *filp,
+			size_t nr, loff_t off,
+			void (*complete)(u64 user_data, long res),
+			u64 user_data)
+{
+	iocb->ki_filp = filp;
+	iocb->ki_nbytes = nr;
+	iocb->ki_pos = off;
+	iocb->ki_ctx = (void *)-1;
+
+	iocb->ki_obj.complete = complete;
+	iocb->ki_user_data = user_data;
+}
+EXPORT_SYMBOL_GPL(aio_kernel_init_rw);
+
+static ssize_t aio_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+	struct file *file = iocb->ki_filp;
+	ssize_t ret;
+
+	if (unlikely(!(file->f_mode & FMODE_READ)))
+		return -EBADF;
+
+	ret = security_file_permission(file, MAY_READ);
+	if (unlikely(ret))
+		return ret;
+
+	if (!file->f_op->read_iter)
+		return -EINVAL;
+
+	return file->f_op->read_iter(iocb, iter);
+}
+
+static ssize_t aio_write_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+	struct file *file = iocb->ki_filp;
+	ssize_t ret;
+
+	if (unlikely(!(file->f_mode & FMODE_WRITE)))
+		return -EBADF;
+
+	ret = security_file_permission(file, MAY_WRITE);
+	if (unlikely(ret))
+		return ret;
+
+	if (!file->f_op->write_iter)
+		return -EINVAL;
+
+	file_start_write(file);
+	ret = file->f_op->write_iter(iocb, iter);
+	file_end_write(file);
+
+	return ret;
+}
+
+/*
+ * The iocb is our responsibility once this is called.  The caller must not
+ * reference it.
+ *
+ * Callers must be prepared for their iocb completion callback to be called the
+ * moment they enter this function.  The completion callback may be called from
+ * any context.
+ *
+ * Returns: 0: the iocb completion callback will be called with the op result
+ * negative errno: the operation was not submitted and the iocb was freed
+ */
+int aio_kernel_submit(struct kiocb *iocb, unsigned op,
+		      struct iov_iter *iter)
+{
+	int ret = -EINVAL;
+
+	if (WARN_ON(!is_kernel_kiocb(iocb) || !iocb->ki_obj.complete
+			|| !iocb->ki_filp || !(iter->type & ITER_BVEC)))
+		return ret;
+
+	if (op == IOCB_CMD_READ_ITER)
+		ret = aio_read_iter(iocb, iter);
+	else if (op == IOCB_CMD_WRITE_ITER)
+		ret = aio_write_iter(iocb, iter);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(aio_kernel_submit);
+
 static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
 			 struct iocb *iocb, bool compat)
 {
diff --git a/include/linux/aio.h b/include/linux/aio.h
index d9c92da..c68504d 100644
--- a/include/linux/aio.h
+++ b/include/linux/aio.h
@@ -14,6 +14,12 @@ struct kiocb;
 
 #define KIOCB_KEY		0
 
+/* opcode values not exposed to user space */
+enum {
+	IOCB_CMD_READ_ITER = 0x10000,
+	IOCB_CMD_WRITE_ITER = 0x10001,
+};
+
 /*
  * We use ki_cancel == KIOCB_CANCELLED to indicate that a kiocb has been either
  * cancelled or completed (this makes a certain amount of sense because
@@ -31,13 +37,15 @@ typedef int (kiocb_cancel_fn)(struct kiocb *);
 
 struct kiocb {
 	struct file		*ki_filp;
-	struct kioctx		*ki_ctx;	/* NULL for sync ops */
+	struct kioctx		*ki_ctx;	/* NULL for sync ops,
+						 * -1 for kernel caller */
 	kiocb_cancel_fn		*ki_cancel;
 	void			*private;
 
 	union {
 		void __user		*user;
 		struct task_struct	*tsk;
+		void			(*complete)(u64 user_data, long res);
 	} ki_obj;
 
 	__u64			ki_user_data;	/* user's data for completion */
@@ -59,6 +67,11 @@ static inline bool is_sync_kiocb(struct kiocb *kiocb)
 	return kiocb->ki_ctx == NULL;
 }
 
+static inline bool is_kernel_kiocb(struct kiocb *kiocb)
+{
+	return kiocb->ki_ctx == (void *)-1;
+}
+
 static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
 {
 	*kiocb = (struct kiocb) {
@@ -77,6 +90,12 @@ extern void exit_aio(struct mm_struct *mm);
 extern long do_io_submit(aio_context_t ctx_id, long nr,
 			 struct iocb __user *__user *iocbpp, bool compat);
 void kiocb_set_cancel_fn(struct kiocb *req, kiocb_cancel_fn *cancel);
+struct kiocb *aio_kernel_alloc(gfp_t gfp, unsigned extra);
+void aio_kernel_free(struct kiocb *iocb);
+void aio_kernel_init_rw(struct kiocb *iocb, struct file *filp, size_t nr,
+			loff_t off, void (*complete)(u64 user_data, long res),
+			u64 user_data);
+int aio_kernel_submit(struct kiocb *iocb, unsigned op, struct iov_iter *iter);
 #else
 static inline ssize_t wait_on_sync_kiocb(struct kiocb *iocb) { return 0; }
 static inline void aio_complete(struct kiocb *iocb, long res, long res2) { }
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

[PATCH 8/9] fd/direct-io: introduce should_dirty for kernel aio

[Ming Lei]

For pages from kernel AIO, it is required to dirty
them before direct IO.

The idea is borrowd from Dave previous post.

Cc: Zach Brown <zab@zabbo.net>
Cc: Dave Kleikamp <dave.kleikamp@oracle.com>
Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 fs/direct-io.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 3997023..bc3212ee 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -122,6 +122,7 @@ struct dio {
 	int page_errors;		/* errno from get_user_pages() */
 	int is_async;			/* is IO async ? */
 	bool defer_completion;		/* defer AIO completion to workqueue? */
+	bool should_dirty;		/* if page should be dirty */
 	int io_error;			/* IO error in completion path */
 	unsigned long refcount;		/* direct_io_worker() and bios */
 	struct bio *bio_list;		/* singly linked via bi_private */
@@ -397,7 +398,7 @@ static inline void dio_bio_submit(struct dio *dio, struct dio_submit *sdio)
 	dio->refcount++;
 	spin_unlock_irqrestore(&dio->bio_lock, flags);
 
-	if (dio->is_async && dio->rw == READ)
+	if (dio->is_async && dio->rw == READ && dio->should_dirty)
 		bio_set_pages_dirty(bio);
 
 	if (sdio->submit_io)
@@ -468,13 +469,14 @@ static int dio_bio_complete(struct dio *dio, struct bio *bio)
 	if (!uptodate)
 		dio->io_error = -EIO;
 
-	if (dio->is_async && dio->rw == READ) {
+	if (dio->is_async && dio->rw == READ && dio->should_dirty) {
 		bio_check_pages_dirty(bio);	/* transfers ownership */
 	} else {
 		bio_for_each_segment_all(bvec, bio, i) {
 			struct page *page = bvec->bv_page;
 
-			if (dio->rw == READ && !PageCompound(page))
+			if (dio->rw == READ && !PageCompound(page) &&
+					dio->should_dirty)
 				set_page_dirty_lock(page);
 			page_cache_release(page);
 		}
@@ -1217,6 +1219,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
 
 	spin_lock_init(&dio->bio_lock);
 	dio->refcount = 1;
+	dio->should_dirty = !is_kernel_kiocb(iocb);
 
 	sdio.iter = iter;
 	sdio.final_block_in_request =
-- 
1.7.9.5

Re: [PATCH 7/9] aio: add aio_kernel_() interface

[Zach Brown]

On Thu, Jul 24, 2014 at 06:55:28AM +0800, Ming Lei wrote:
> From: Dave Kleikamp <dave.kleikamp@oracle.com>
> 
> This adds an interface that lets kernel callers submit aio iocbs without
> going through the user space syscalls.  This lets kernel callers avoid
> the management limits and overhead of the context.  It will also let us
> integrate aio operations with other kernel apis that the user space
> interface doesn't have access to.
> 
> This patch is based on Dave's posts in below links:
> 
> 	https://lkml.org/lkml/2013/10/16/365
> 	https://groups.google.com/forum/#!topic/linux.kernel/l7mogGJZoKQ

This was originally written a billion years ago when dinosaurs roamed
the earth.  Also, notably, before Kent and Ben reworked a bunch of the
aio core.  I'd want them to take a look at this patch to make sure that
it doesn't rely on any assumptions that have changed.

> +/* opcode values not exposed to user space */
> +enum {
> +	IOCB_CMD_READ_ITER = 0x10000,
> +	IOCB_CMD_WRITE_ITER = 0x10001,
> +};

And I think the consensus was that this isn't good enough.  Find a way
to encode the kernel caller ops without polluting the uiocb cmd name
space.

(I've now come to think that this entire approach of having loop use aio
is misguided and that the way forward is to have dio consume what loop
naturally produces -- bios, blk-mq rqs, whatever -- but I'm on to other
things these days.)

- z

Re: [PATCH 7/9] aio: add aio_kernel_() interface

[Ming Lei]

On Thu, Jul 24, 2014 at 7:16 AM, Zach Brown <zab@zabbo.net> wrote:
> On Thu, Jul 24, 2014 at 06:55:28AM +0800, Ming Lei wrote:
>> From: Dave Kleikamp <dave.kleikamp@oracle.com>
>>
>> This adds an interface that lets kernel callers submit aio iocbs without
>> going through the user space syscalls.  This lets kernel callers avoid
>> the management limits and overhead of the context.  It will also let us
>> integrate aio operations with other kernel apis that the user space
>> interface doesn't have access to.
>>
>> This patch is based on Dave's posts in below links:
>>
>>       https://lkml.org/lkml/2013/10/16/365
>>       https://groups.google.com/forum/#!topic/linux.kernel/l7mogGJZoKQ
>
> This was originally written a billion years ago when dinosaurs roamed
> the earth.  Also, notably, before Kent and Ben reworked a bunch of the

Not so far away, this patch is based on Dave's last version of V9, which
was posted in Oct, 2013, :-)

> aio core.  I'd want them to take a look at this patch to make sure that
> it doesn't rely on any assumptions that have changed.

Looks I missed to Cc Ken, :-(

>
>> +/* opcode values not exposed to user space */
>> +enum {
>> +     IOCB_CMD_READ_ITER = 0x10000,
>> +     IOCB_CMD_WRITE_ITER = 0x10001,
>> +};
>
> And I think the consensus was that this isn't good enough.  Find a way
> to encode the kernel caller ops without polluting the uiocb cmd name
> space.

That is easy, since the two cmd names are only for kernel AIO, whatever
should be OK, but looks I didn't see such comment.

>
> (I've now come to think that this entire approach of having loop use aio
> is misguided and that the way forward is to have dio consume what loop
> naturally produces -- bios, blk-mq rqs, whatever -- but I'm on to other

Yes, that is what these patches are doing, and actually AIO's
model is a good match to driver's interface. Lots of drivers
use the asynchronous model(submit, complete, ...).

> things these days.)

At least, loop can improve its throughput much by kernel AIO
without big changes to fs/direct-io(attribute much to ITER_BVEC),
and vhost-scsi should benefit from it too.

Thanks,

Re: [PATCH 7/9] aio: add aio_kernel_() interface

[Dave Kleikamp]

On 07/23/2014 08:57 PM, Ming Lei wrote:
> On Thu, Jul 24, 2014 at 7:16 AM, Zach Brown <zab@zabbo.net> wrote:
>> On Thu, Jul 24, 2014 at 06:55:28AM +0800, Ming Lei wrote:
>>> From: Dave Kleikamp <dave.kleikamp@oracle.com>
>>>
>>> This adds an interface that lets kernel callers submit aio iocbs without
>>> going through the user space syscalls.  This lets kernel callers avoid
>>> the management limits and overhead of the context.  It will also let us
>>> integrate aio operations with other kernel apis that the user space
>>> interface doesn't have access to.
>>>
>>> This patch is based on Dave's posts in below links:
>>>
>>>       https://lkml.org/lkml/2013/10/16/365
>>>       https://groups.google.com/forum/#!topic/linux.kernel/l7mogGJZoKQ
>>
>> This was originally written a billion years ago when dinosaurs roamed
>> the earth.  Also, notably, before Kent and Ben reworked a bunch of the
> 
> Not so far away, this patch is based on Dave's last version of V9, which
> was posted in Oct, 2013, :-)

Which was based on a much earlier patch from Zach. I regret that I left
aio_kernel_submit entangled with aio_run_iocb when I reworked his patches.

>> aio core.  I'd want them to take a look at this patch to make sure that
>> it doesn't rely on any assumptions that have changed.
> 
> Looks I missed to Cc Ken, :-(
> 
>>
>>> +/* opcode values not exposed to user space */
>>> +enum {
>>> +     IOCB_CMD_READ_ITER = 0x10000,
>>> +     IOCB_CMD_WRITE_ITER = 0x10001,
>>> +};
>>
>> And I think the consensus was that this isn't good enough.  Find a way
>> to encode the kernel caller ops without polluting the uiocb cmd name
>> space.
> 
> That is easy, since the two cmd names are only for kernel AIO, whatever
> should be OK, but looks I didn't see such comment.

Agreed. These were added because the flags had been interpreted by
aio_run_iocb(). I'm happy that is no longer the case.

>>
>> (I've now come to think that this entire approach of having loop use aio
>> is misguided and that the way forward is to have dio consume what loop
>> naturally produces -- bios, blk-mq rqs, whatever -- but I'm on to other
> 
> Yes, that is what these patches are doing, and actually AIO's
> model is a good match to driver's interface. Lots of drivers
> use the asynchronous model(submit, complete, ...).
> 
>> things these days.)
> 
> At least, loop can improve its throughput much by kernel AIO
> without big changes to fs/direct-io(attribute much to ITER_BVEC),
> and vhost-scsi should benefit from it too.
> 
> Thanks,
> 

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

Re: [PATCH 7/9] aio: add aio_kernel_() interface

[Ming Lei]

On Thu, Jul 24, 2014 at 11:16 PM, Dave Kleikamp
<dave.kleikamp@oracle.com> wrote:
> On 07/23/2014 08:57 PM, Ming Lei wrote:
>> On Thu, Jul 24, 2014 at 7:16 AM, Zach Brown <zab@zabbo.net> wrote:
>>> On Thu, Jul 24, 2014 at 06:55:28AM +0800, Ming Lei wrote:
>>>> From: Dave Kleikamp <dave.kleikamp@oracle.com>
>>>>
>>>> This adds an interface that lets kernel callers submit aio iocbs without
>>>> going through the user space syscalls.  This lets kernel callers avoid
>>>> the management limits and overhead of the context.  It will also let us
>>>> integrate aio operations with other kernel apis that the user space
>>>> interface doesn't have access to.
>>>>
>>>> This patch is based on Dave's posts in below links:
>>>>
>>>>       https://lkml.org/lkml/2013/10/16/365
>>>>       https://groups.google.com/forum/#!topic/linux.kernel/l7mogGJZoKQ
>>>
>>> This was originally written a billion years ago when dinosaurs roamed
>>> the earth.  Also, notably, before Kent and Ben reworked a bunch of the
>>
>> Not so far away, this patch is based on Dave's last version of V9, which
>> was posted in Oct, 2013, :-)
>
> Which was based on a much earlier patch from Zach. I regret that I left
> aio_kernel_submit entangled with aio_run_iocb when I reworked his patches.
>
>>> aio core.  I'd want them to take a look at this patch to make sure that
>>> it doesn't rely on any assumptions that have changed.
>>
>> Looks I missed to Cc Ken, :-(
>>
>>>
>>>> +/* opcode values not exposed to user space */
>>>> +enum {
>>>> +     IOCB_CMD_READ_ITER = 0x10000,
>>>> +     IOCB_CMD_WRITE_ITER = 0x10001,
>>>> +};
>>>
>>> And I think the consensus was that this isn't good enough.  Find a way
>>> to encode the kernel caller ops without polluting the uiocb cmd name
>>> space.
>>
>> That is easy, since the two cmd names are only for kernel AIO, whatever
>> should be OK, but looks I didn't see such comment.
>
> Agreed. These were added because the flags had been interpreted by
> aio_run_iocb(). I'm happy that is no longer the case.

We can remove the two cmd names completely, and just use one
read/write flag, will do it in V1.

>
>>>
>>> (I've now come to think that this entire approach of having loop use aio
>>> is misguided and that the way forward is to have dio consume what loop
>>> naturally produces -- bios, blk-mq rqs, whatever -- but I'm on to other
>>
>> Yes, that is what these patches are doing, and actually AIO's
>> model is a good match to driver's interface. Lots of drivers
>> use the asynchronous model(submit, complete, ...).
>>
>>> things these days.)
>>
>> At least, loop can improve its throughput much by kernel AIO
>> without big changes to fs/direct-io(attribute much to ITER_BVEC),
>> and vhost-scsi should benefit from it too.
>>
>> Thanks,
>>