Add support for async buffered reads
Subsystems (8)
Jens Axboe
maintainerv2 cover Add support for async buffered reads
We technically support this already through io_uring, but it's implemented with a thread backend to support cases where we would block. This isn't ideal.
After a few prep patches, the core of this patchset is adding support for async callbacks on page unlock. With this primitive, we can simply retry the IO operation. With io_uring, this works a lot like poll based retry for files that support it. If a page is currently locked and needed, -EIOCBQUEUED is returned with a callback armed. The callers callback is responsible for restarting the operation.
With this callback primitive, we can add support for generic_file_buffered_read(), which is what most file systems end up using for buffered reads. XFS/ext4/btrfs/bdev is wired up, but probably trivial to add more.
The file flags support for this by setting FMODE_BUF_RASYNC, similar to what we do for FMODE_NOWAIT. Open to suggestions here if this is the preferred method or not.
In terms of results, I wrote a small test app that randomly reads 4G of data in 4K chunks from a file hosted by ext4. The app uses a queue depth of 32.
preadv for comparison:
real 1m13.821s user 0m0.558s sys 0m11.125s CPU ~13%
Mainline:
real 0m12.054s user 0m0.111s sys 0m5.659s CPU ~32% + ~50% == ~82%
This patchset:
real 0m9.283s user 0m0.147s sys 0m4.619s CPU ~52%
The CPU numbers are just a rough estimate. For the mainline io_uring run, this includes the app itself and all the threads doing IO on its behalf (32% for the app, ~1.6% per worker and 32 of them). Context switch rate is much smaller with the patchset, since we only have the one task performing IO.
The goal here is efficiency. Async thread offload adds latency, and it also adds noticable overhead on items such as adding pages to the page cache. By allowing proper async buffered read support, we don't have X threads hammering on the same inode page cache, we have just the single app actually doing IO.
Series can also be found here:
https://git.kernel.dk/cgit/linux-block/log/?h=async-buffered.2
or pull from:
git://git.kernel.dk/linux-block async-buffered.2
fs/block_dev.c | 2 +- fs/btrfs/file.c | 2 +- fs/ext4/file.c | 2 +- fs/io_uring.c | 102 ++++++++++++++++++++++++++++++++++++++ fs/xfs/xfs_file.c | 2 +- include/linux/blk_types.h | 3 +- include/linux/fs.h | 5 ++ include/linux/pagemap.h | 39 +++++++++++++++ mm/filemap.c | 83 +++++++++++++++++++++++++------ 9 files changed, 219 insertions(+), 21 deletions(-)
Changes since v1:
- Fix an issue with inline page locking
- Fix a potential race with __wait_on_page_locked_async()
- Fix a hang related to not setting page_match, thus missing a wakeup
Signature · 1 line
Jens Axboe
Jens Axboe
maintainerPATCH v2 1/11 block: read-ahead submission should imply no-wait as well
As read-ahead is opportunistic, don't block for request allocation.
--- include/linux/blk_types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
include/linux/blk_types.h +2 -1
@@ -374,7 +374,8 @@ enum req_flag_bits {#define REQ_INTEGRITY (1ULL << __REQ_INTEGRITY)#define REQ_FUA (1ULL << __REQ_FUA)#define REQ_PREFLUSH (1ULL << __REQ_PREFLUSH) -#define REQ_RAHEAD (1ULL << __REQ_RAHEAD) +#define REQ_RAHEAD \ + ((1ULL << __REQ_RAHEAD) | (1ULL << __REQ_NOWAIT))#define REQ_BACKGROUND (1ULL << __REQ_BACKGROUND)#define REQ_NOWAIT (1ULL << __REQ_NOWAIT)#define REQ_CGROUP_PUNT (1ULL << __REQ_CGROUP_PUNT)
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 2/11 mm: allow read-ahead with IOCB_NOWAIT set
In reply to Jens AxboeThe read-ahead shouldn't block, so allow it to be done even if IOCB_NOWAIT is set in the kiocb.
--- mm/filemap.c | 2 -- 1 file changed, 2 deletions(-)
mm/filemap.c +0 -2
@@ -2031,8 +2031,6 @@ static ssize_t generic_file_buffered_read(struct kiocb *iocb, page =find_get_page (mapping, index);if (!page) { -if (iocb->ki_flags &IOCB_NOWAIT ) - goto would_block;page_cache_sync_readahead (mapping, ra, filp, index, last_index - index);
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 3/11 mm: add support for async page locking
In reply to Jens AxboeNormally waiting for a page to become unlocked, or locking the page, requires waiting for IO to complete. Add support for lock_page_async() and wait_on_page_locked_async(), which are callback based instead. This allows a caller to get notified when a page becomes unlocked, rather than wait for it.
We use the iocb->private field to pass in this necessary data for this to happen. struct wait_page_key is made public, and we define struct wait_page_async as the interface between the caller and the core.
--- include/linux/fs.h | 2 ++ include/linux/pagemap.h | 21 ++++++++++++++++ mm/filemap.c | 56 +++++++++++++++++++++++++++++++++++------ 3 files changed, 72 insertions(+), 7 deletions(-)
include/linux/fs.h +2 -0
@@ -314,6 +314,8 @@ enum rw_hint {#define IOCB_SYNC (1 << 5)#define IOCB_WRITE (1 << 6)#define IOCB_NOWAIT (1 << 7) +/* iocb->private holds wait_page_async struct */ +#define IOCB_WAITQ (1 << 8)struct kiocb {struct file *ki_filp;
include/linux/pagemap.h +21 -0
@@ -456,8 +456,21 @@ static inline pgoff_t linear_page_index(struct vm_area_struct *vma,return pgoff; } +/* This has the same layout as wait_bit_key - see fs/cachefiles/rdwr.c */ +struct wait_page_key { +struct page *page; +int bit_nr; +int page_match; +}; + +struct wait_page_async { +struct wait_queue_entry wait; +struct wait_page_key key; +}; +extern void __lock_page (struct page *page);extern int __lock_page_killable (struct page *page); +extern int __lock_page_async (struct page *page,struct wait_page_async *wait);extern int __lock_page_or_retry (struct page *page,struct mm_struct *mm,unsigned int flags);extern void unlock_page (struct page *page);
@@ -494,6 +507,14 @@ static inline int lock_page_killable(struct page *page)return 0 ; } +static inline int lock_page_async (struct page *page, +struct wait_page_async *wait) +{ +if (!trylock_page (page)) +return __lock_page_async (page, wait); +return 0 ; +} +/* * lock_page_or_retry - Lock the page, unless this would block and the * caller indicated that it can handle a retry.
mm/filemap.c +49 -7
74 lines not on this page · raw message
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 4/11 mm: support async buffered reads in generic_file_buffered_read()
In reply to Jens AxboeUse the async page locking infrastructure, if IOCB_WAITQ is set in the passed in iocb. The caller must expect an -EIOCBQUEUED return value, which means that IO is started but not done yet. This is similar to how O_DIRECT signals the same operation. Once the callback is received by the caller for IO completion, the caller must retry the operation.
--- mm/filemap.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
mm/filemap.c +18 -7
@@ -2086,17 +2086,25 @@ static ssize_t generic_file_buffered_read(struct kiocb *iocb, index, last_index - index); }if (!PageUptodate (page)) { -if (iocb->ki_flags &IOCB_NOWAIT ) { -put_page (page); - goto would_block; - } -/* * See comment in do_read_cache_page on why * wait_on_page_locked is used to avoid unnecessarily * serialisations and why it's safe. */ - error =wait_on_page_locked_killable (page); +if (iocb->ki_flags &IOCB_WAITQ ) { +if (written) { +put_page (page); + goto out; + } + error =wait_on_page_locked_async (page, + iocb->private); + }else { +if (iocb->ki_flags &IOCB_NOWAIT ) { +put_page (page); + goto would_block; + } + error =wait_on_page_locked_killable (page); + }if (unlikely (error)) goto readpage_error;if (PageUptodate (page))
@@ -2184,7 +2192,10 @@ static ssize_t generic_file_buffered_read(struct kiocb *iocb, page_not_up_to_date:/* Get exclusive access to the page ... */ - error =lock_page_killable (page); +if (iocb->ki_flags &IOCB_WAITQ ) + error =lock_page_async (page, iocb->private); +else + error =lock_page_killable (page);if (unlikely (error)) goto readpage_error;
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 5/11 fs: add FMODE_BUF_RASYNC
In reply to Jens AxboeIf set, this indicates that the file system supports IOCB_WAITQ for buffered reads.
--- include/linux/fs.h | 3 +++ 1 file changed, 3 insertions(+)
include/linux/fs.h +3 -0
@@ -175,6 +175,9 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,/* File does not contribute to nr_files count */ #define FMODE_NOACCOUNT ((__force fmode_t)0x20000000) +/* File supports async buffered reads */ +#define FMODE_BUF_RASYNC ((__force fmode_t)0x40000000) +/* * Flag for rw_copy_check_uvector and compat_rw_copy_check_uvector * that indicates that they should check the contents of the iovec are
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 6/11 ext4: flag as supporting buffered async reads
In reply to Jens Axboe--- fs/ext4/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ext4/file.c +1 -1
@@ -826,7 +826,7 @@ static int ext4_file_open(struct inode * inode, struct file * filp)return ret; } - filp->f_mode |=FMODE_NOWAIT ; + filp->f_mode |=FMODE_NOWAIT |FMODE_BUF_RASYNC ;return dquot_file_open (inode, filp); }
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 7/11 block: flag block devices as supporting IOCB_WAITQ
In reply to Jens Axboe--- fs/block_dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/block_dev.c +1 -1
@@ -1851,7 +1851,7 @@ static int blkdev_open(struct inode * inode, struct file * filp)*/ filp->f_flags |=O_LARGEFILE ; - filp->f_mode |=FMODE_NOWAIT ; + filp->f_mode |=FMODE_NOWAIT |FMODE_BUF_RASYNC ;if (filp->f_flags &O_NDELAY ) filp->f_mode |=FMODE_NDELAY ;
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 8/11 xfs: flag files as supporting buffered async reads
In reply to Jens AxboeXFS uses generic_file_read_iter(), which already supports this.
--- fs/xfs/xfs_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_file.c +1 -1
@@ -1080,7 +1080,7 @@ xfs_file_open(return -EFBIG ;if (XFS_FORCED_SHUTDOWN (XFS_M (inode->i_sb)))return -EIO ; - file->f_mode |=FMODE_NOWAIT ; + file->f_mode |=FMODE_NOWAIT |FMODE_BUF_RASYNC ;return 0 ; }
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 9/11 btrfs: flag files as supporting buffered async reads
In reply to Jens Axboebtrfs uses generic_file_read_iter(), which already supports this.
--- fs/btrfs/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/file.c +1 -1
@@ -3480,7 +3480,7 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int whence)static int btrfs_file_open (struct inode *inode,struct file *filp) { - filp->f_mode |=FMODE_NOWAIT ; + filp->f_mode |=FMODE_NOWAIT |FMODE_BUF_RASYNC ;return generic_file_open (inode, filp); }
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 10/11 mm: add kiocb_wait_page_async_init() helper
In reply to Jens AxboeChecks if the file supports it, and initializes the values that we need. Caller passes in 'data' pointer, if any, and the callback function to be used.
--- include/linux/pagemap.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
include/linux/pagemap.h +18 -0
@@ -468,6 +468,24 @@ struct wait_page_async {struct wait_page_key key; }; +static inline int kiocb_wait_page_async_init (struct kiocb *kiocb, +struct wait_page_async *wait, +wait_queue_func_t func, +void *data) +{ +if (kiocb->ki_filp->f_mode &FMODE_BUF_RASYNC ) { + wait->wait.func = func; + wait->wait.private = data; + wait->wait.flags =0 ; +INIT_LIST_HEAD (&wait->wait.entry); + kiocb->ki_flags |=IOCB_WAITQ ; + kiocb->private = wait; +return 0 ; + } + +return -EOPNOTSUPP ; +} +extern void __lock_page (struct page *page);extern int __lock_page_killable (struct page *page);extern int __lock_page_async (struct page *page,struct wait_page_async *wait);
Signature · 1 line
2.26.2
Jens Axboe
maintainerPATCH v2 11/11 io_uring: support true async buffered reads, if file provides it
In reply to Jens AxboeIf the file is flagged with FMODE_BUF_RASYNC, then we don't have to punt the buffered read to an io-wq worker. Instead we can rely on page unlocking callbacks to support retry based async IO. This is a lot more efficient than doing async thread offload.
The retry is done similarly to how we handle poll based retry. From the unlock callback, we simply queue the retry to a task_work based handler.
--- fs/io_uring.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
fs/io_uring.c +102 -0
@@ -498,6 +498,8 @@ struct io_async_rw {struct iovec *iov;ssize_t nr_segs;ssize_t size; +struct wait_page_async wait; +struct callback_head task_work; };struct io_async_ctx {
@@ -2568,6 +2570,102 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe,return 0 ; } +static void io_async_buf_cancel (struct callback_head *cb) +{ +struct io_async_rw *rw; +struct io_ring_ctx *ctx; +struct io_kiocb *req; + + rw = container_of(cb, struct io_async_rw, task_work); + req = rw->wait.wait.private; + ctx = req->ctx; + +spin_lock_irq (&ctx->completion_lock); +io_cqring_fill_event (req, -ECANCELED ); +io_commit_cqring (ctx); +spin_unlock_irq (&ctx->completion_lock); + +io_cqring_ev_posted (ctx); +req_set_fail_links (req); +io_double_put_req (req); +} + +static void io_async_buf_retry (struct callback_head *cb) +{ +struct io_async_rw *rw; +struct io_ring_ctx *ctx; +struct io_kiocb *req; + + rw = container_of(cb, struct io_async_rw, task_work); + req = rw->wait.wait.private; + ctx = req->ctx; + +__set_current_state (TASK_RUNNING ); +mutex_lock (&ctx->uring_lock); +__io_queue_sqe (req,NULL ); +mutex_unlock (&ctx->uring_lock); +} + +static int io_async_buf_func (struct wait_queue_entry *wait,unsigned mode, +int sync,void *arg) +{ +struct wait_page_async *wp; +struct io_kiocb *req = wait->private; +struct io_async_rw *rw = &req->io->rw; +struct wait_page_key *key = arg; +struct task_struct *tsk; +int ret; + + wp = container_of(wait, struct wait_page_async, wait); +if (wp->key.page != key->page) +return 0 ; + key->page_match =1 ; +if (wp->key.bit_nr != key->bit_nr) +return 0 ; +if (test_bit (PG_locked, &key->page->flags)) +return -1 ; + +list_del_init (&wait->entry); + +init_task_work (&rw->task_work, io_async_buf_retry); +/* submit ref gets dropped, acquire a new one */ +refcount_inc (&req->refs); + tsk = req->task; + ret =task_work_add (tsk, &rw->task_work, true); +if (unlikely (ret)) { +/* queue just for cancelation */ +init_task_work (&rw->task_work, io_async_buf_cancel); + tsk =io_wq_get_task (req->ctx->io_wq); +task_work_add (tsk, &rw->task_work, true); + } +wake_up_process (tsk); +return 1 ; +} + +static bool io_rw_should_retry (struct io_kiocb *req) +{ +struct kiocb *kiocb = &req->rw.kiocb; +int ret; + +/* already tried, or we're doing O_DIRECT */ +if (kiocb->ki_flags & (IOCB_DIRECT |IOCB_WAITQ )) +return false; +/* +* just use poll if we can, and don't attempt if the fs doesn't +* support callback based unlocks +*/ +if (file_can_poll (req->file) || !(req->file->f_mode &FMODE_BUF_RASYNC )) +return false; + + ret =kiocb_wait_page_async_init (kiocb, &req->io->rw.wait, + io_async_buf_func, req); +if (ret) +return false; +get_task_struct (current); + req->task = current; +return true; +} +static int io_read (struct io_kiocb *req,bool force_nonblock) {struct iovec inline_vecs[UIO_FASTIOV ], *iovec = inline_vecs;
@@ -2601,6 +2699,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock)if (!ret) {ssize_t ret2; +retry:if (req->file->f_op->read_iter) ret2 =call_read_iter (req->file, kiocb, &iter);else
@@ -2619,6 +2718,9 @@ static int io_read(struct io_kiocb *req, bool force_nonblock)if (!(req->flags &REQ_F_NOWAIT ) && !file_can_poll (req->file)) req->flags |=REQ_F_MUST_PUNT ; +if (io_rw_should_retry (req)) + goto retry; + kiocb->ki_flags &= ~IOCB_WAITQ ;return -EAGAIN ; } }
Signature · 1 line
2.26.2