* [PATCH 7/9] aio: add aio_kernel_() interface [not found] <1406156130-15575-1-git-send-email-ming.lei@canonical.com> @ 2014-07-23 22:55 ` Ming Lei 2014-07-23 23:16 ` Zach Brown 2014-07-23 22:55 ` [PATCH 8/9] fd/direct-io: introduce should_dirty for kernel aio Ming Lei 1 sibling, 1 reply; 6+ messages in thread From: Ming Lei @ 2014-07-23 22:55 UTC (permalink / raw) To: Jens Axboe, linux-kernel Cc: Andrew Morton, Zach Brown, Dave Kleikamp, Benjamin LaHaise, Alexander Viro, linux-fsdevel, open list:AIO, 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> ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 7/9] aio: add aio_kernel_() interface 2014-07-23 22:55 ` [PATCH 7/9] aio: add aio_kernel_() interface Ming Lei @ 2014-07-23 23:16 ` Zach Brown 2014-07-24 1:57 ` Ming Lei 0 siblings, 1 reply; 6+ messages in thread From: Zach Brown @ 2014-07-23 23:16 UTC (permalink / raw) To: Ming Lei Cc: Jens Axboe, linux-kernel, Andrew Morton, Dave Kleikamp, Benjamin LaHaise, Alexander Viro, linux-fsdevel, open list:AIO 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 7/9] aio: add aio_kernel_() interface 2014-07-23 23:16 ` Zach Brown @ 2014-07-24 1:57 ` Ming Lei 2014-07-24 15:16 ` Dave Kleikamp 0 siblings, 1 reply; 6+ messages in thread From: Ming Lei @ 2014-07-24 1:57 UTC (permalink / raw) To: Zach Brown Cc: Jens Axboe, Linux Kernel Mailing List, Andrew Morton, Dave Kleikamp, Benjamin LaHaise, Alexander Viro, Linux FS Devel, open list:AIO, Kent Overstreet 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, ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 7/9] aio: add aio_kernel_() interface 2014-07-24 1:57 ` Ming Lei @ 2014-07-24 15:16 ` Dave Kleikamp 2014-07-25 1:25 ` Ming Lei 0 siblings, 1 reply; 6+ messages in thread From: Dave Kleikamp @ 2014-07-24 15:16 UTC (permalink / raw) To: Ming Lei, Zach Brown Cc: Jens Axboe, Linux Kernel Mailing List, Andrew Morton, Benjamin LaHaise, Alexander Viro, Linux FS Devel, open, list 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> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 7/9] aio: add aio_kernel_() interface 2014-07-24 15:16 ` Dave Kleikamp @ 2014-07-25 1:25 ` Ming Lei 0 siblings, 0 replies; 6+ messages in thread From: Ming Lei @ 2014-07-25 1:25 UTC (permalink / raw) To: Dave Kleikamp Cc: Zach Brown, Jens Axboe, Linux Kernel Mailing List, Andrew Morton, Benjamin LaHaise, Alexander Viro, Linux FS Devel, open list:AIO <linux-aio@kvack.org>, Kent Overstreet 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, >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 8/9] fd/direct-io: introduce should_dirty for kernel aio [not found] <1406156130-15575-1-git-send-email-ming.lei@canonical.com> 2014-07-23 22:55 ` [PATCH 7/9] aio: add aio_kernel_() interface Ming Lei @ 2014-07-23 22:55 ` Ming Lei 1 sibling, 0 replies; 6+ messages in thread From: Ming Lei @ 2014-07-23 22:55 UTC (permalink / raw) To: Jens Axboe, linux-kernel Cc: Andrew Morton, Zach Brown, Dave Kleikamp, Benjamin LaHaise, Ming Lei, Alexander Viro, linux-fsdevel 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-25 1:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1406156130-15575-1-git-send-email-ming.lei@canonical.com>
2014-07-23 22:55 ` [PATCH 7/9] aio: add aio_kernel_() interface Ming Lei
2014-07-23 23:16 ` Zach Brown
2014-07-24 1:57 ` Ming Lei
2014-07-24 15:16 ` Dave Kleikamp
2014-07-25 1:25 ` Ming Lei
2014-07-23 22:55 ` [PATCH 8/9] fd/direct-io: introduce should_dirty for kernel aio Ming Lei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).