From cfb1e33eed48165763edc7a4a067cf5f74898d0b Mon Sep 17 00:00:00 2001 From: Jeff Moyer Date: Fri, 2 Oct 2009 18:57:36 -0400 Subject: aio: implement request batching Hi, Some workloads issue batches of small I/O, and the performance is poor due to the call to blk_run_address_space for every single iocb. Nathan Roberts pointed this out, and suggested that by deferring this call until all I/Os in the iocb array are submitted to the block layer, we can realize some impressive performance gains (up to 30% for sequential 4k reads in batches of 16). Signed-off-by: Jeff Moyer Signed-off-by: Jens Axboe --- fs/aio.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) (limited to 'fs/aio.c') diff --git a/fs/aio.c b/fs/aio.c index 02a2c9340573..cf0bef428f88 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -32,6 +32,9 @@ #include #include #include +#include +#include +#include #include #include @@ -60,6 +63,14 @@ static DECLARE_WORK(fput_work, aio_fput_routine); static DEFINE_SPINLOCK(fput_lock); static LIST_HEAD(fput_head); +#define AIO_BATCH_HASH_BITS 3 /* allocated on-stack, so don't go crazy */ +#define AIO_BATCH_HASH_SIZE (1 << AIO_BATCH_HASH_BITS) +struct aio_batch_entry { + struct hlist_node list; + struct address_space *mapping; +}; +mempool_t *abe_pool; + static void aio_kick_handler(struct work_struct *); static void aio_queue_work(struct kioctx *); @@ -73,6 +84,8 @@ static int __init aio_setup(void) kioctx_cachep = KMEM_CACHE(kioctx,SLAB_HWCACHE_ALIGN|SLAB_PANIC); aio_wq = create_workqueue("aio"); + abe_pool = mempool_create_kmalloc_pool(1, sizeof(struct aio_batch_entry)); + BUG_ON(!abe_pool); pr_debug("aio_setup: sizeof(struct page) = %d\n", (int)sizeof(struct page)); @@ -1531,8 +1544,44 @@ static int aio_wake_function(wait_queue_t *wait, unsigned mode, return 1; } +static void aio_batch_add(struct address_space *mapping, + struct hlist_head *batch_hash) +{ + struct aio_batch_entry *abe; + struct hlist_node *pos; + unsigned bucket; + + bucket = hash_ptr(mapping, AIO_BATCH_HASH_BITS); + hlist_for_each_entry(abe, pos, &batch_hash[bucket], list) { + if (abe->mapping == mapping) + return; + } + + abe = mempool_alloc(abe_pool, GFP_KERNEL); + BUG_ON(!igrab(mapping->host)); + abe->mapping = mapping; + hlist_add_head(&abe->list, &batch_hash[bucket]); + return; +} + +static void aio_batch_free(struct hlist_head *batch_hash) +{ + struct aio_batch_entry *abe; + struct hlist_node *pos, *n; + int i; + + for (i = 0; i < AIO_BATCH_HASH_SIZE; i++) { + hlist_for_each_entry_safe(abe, pos, n, &batch_hash[i], list) { + blk_run_address_space(abe->mapping); + iput(abe->mapping->host); + hlist_del(&abe->list); + mempool_free(abe, abe_pool); + } + } +} + static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb, - struct iocb *iocb) + struct iocb *iocb, struct hlist_head *batch_hash) { struct kiocb *req; struct file *file; @@ -1608,6 +1657,12 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb, ; } spin_unlock_irq(&ctx->ctx_lock); + if (req->ki_opcode == IOCB_CMD_PREAD || + req->ki_opcode == IOCB_CMD_PREADV || + req->ki_opcode == IOCB_CMD_PWRITE || + req->ki_opcode == IOCB_CMD_PWRITEV) + aio_batch_add(file->f_mapping, batch_hash); + aio_put_req(req); /* drop extra ref to req */ return 0; @@ -1635,6 +1690,7 @@ SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, long, nr, struct kioctx *ctx; long ret = 0; int i; + struct hlist_head batch_hash[AIO_BATCH_HASH_SIZE] = { { 0, }, }; if (unlikely(nr < 0)) return -EINVAL; @@ -1666,10 +1722,11 @@ SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, long, nr, break; } - ret = io_submit_one(ctx, user_iocb, &tmp); + ret = io_submit_one(ctx, user_iocb, &tmp, batch_hash); if (ret) break; } + aio_batch_free(batch_hash); put_ioctx(ctx); return i ? i : ret; -- cgit v1.2.3 From b9d128f1088ea5245109dfc9bbceb128b6371a77 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 29 Oct 2009 13:59:26 +0100 Subject: block: move bdi/address_space unplug functions to backing-dev.h There's nothing block related about them, the backing device is used by things like NFS etc as well. This gets rid of the need to protect such calls by CONFIG_BLOCK. Signed-off-by: Jens Axboe --- fs/aio.c | 1 + include/linux/backing-dev.h | 13 +++++++++++++ include/linux/blkdev.h | 13 ------------- 3 files changed, 14 insertions(+), 13 deletions(-) (limited to 'fs/aio.c') diff --git a/fs/aio.c b/fs/aio.c index cf0bef428f88..c30dfc006108 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #define DEBUG 0 diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h index b449e738533a..fcbc26af00e4 100644 --- a/include/linux/backing-dev.h +++ b/include/linux/backing-dev.h @@ -331,4 +331,17 @@ static inline int bdi_sched_wait(void *word) return 0; } +static inline void blk_run_backing_dev(struct backing_dev_info *bdi, + struct page *page) +{ + if (bdi && bdi->unplug_io_fn) + bdi->unplug_io_fn(bdi, page); +} + +static inline void blk_run_address_space(struct address_space *mapping) +{ + if (mapping) + blk_run_backing_dev(mapping->backing_dev_info, NULL); +} + #endif /* _LINUX_BACKING_DEV_H */ diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 221cecd86bd3..39c601f783a0 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -823,19 +823,6 @@ static inline struct request_queue *bdev_get_queue(struct block_device *bdev) return bdev->bd_disk->queue; } -static inline void blk_run_backing_dev(struct backing_dev_info *bdi, - struct page *page) -{ - if (bdi && bdi->unplug_io_fn) - bdi->unplug_io_fn(bdi, page); -} - -static inline void blk_run_address_space(struct address_space *mapping) -{ - if (mapping) - blk_run_backing_dev(mapping->backing_dev_info, NULL); -} - /* * blk_rq_pos() : the current sector * blk_rq_bytes() : bytes left in the entire request -- cgit v1.2.3 From fac046ad0b1ee2c4244ebf43a26433ef0ea29ae4 Mon Sep 17 00:00:00 2001 From: Shaohua Li Date: Tue, 15 Dec 2009 16:47:47 -0800 Subject: aio: remove unused field Don't know the reason, but it appears ki_wait field of iocb never gets used. Signed-off-by: Shaohua Li Cc: Jeff Moyer Cc: Benjamin LaHaise Cc: Zach Brown Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/aio.c | 40 ++-------------------------------------- include/linux/aio.h | 4 ---- 2 files changed, 2 insertions(+), 42 deletions(-) (limited to 'fs/aio.c') diff --git a/fs/aio.c b/fs/aio.c index c30dfc006108..1cf12b3dd83a 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -711,10 +711,8 @@ static ssize_t aio_run_iocb(struct kiocb *iocb) */ ret = retry(iocb); - if (ret != -EIOCBRETRY && ret != -EIOCBQUEUED) { - BUG_ON(!list_empty(&iocb->ki_wait.task_list)); + if (ret != -EIOCBRETRY && ret != -EIOCBQUEUED) aio_complete(iocb, ret, 0); - } out: spin_lock_irq(&ctx->ctx_lock); @@ -866,13 +864,6 @@ static void try_queue_kicked_iocb(struct kiocb *iocb) unsigned long flags; int run = 0; - /* We're supposed to be the only path putting the iocb back on the run - * list. If we find that the iocb is *back* on a wait queue already - * than retry has happened before we could queue the iocb. This also - * means that the retry could have completed and freed our iocb, no - * good. */ - BUG_ON((!list_empty(&iocb->ki_wait.task_list))); - spin_lock_irqsave(&ctx->ctx_lock, flags); /* set this inside the lock so that we can't race with aio_run_iocb() * testing it and putting the iocb on the run list under the lock */ @@ -886,7 +877,7 @@ static void try_queue_kicked_iocb(struct kiocb *iocb) /* * kick_iocb: * Called typically from a wait queue callback context - * (aio_wake_function) to trigger a retry of the iocb. + * to trigger a retry of the iocb. * The retry is usually executed by aio workqueue * threads (See aio_kick_handler). */ @@ -1520,31 +1511,6 @@ static ssize_t aio_setup_iocb(struct kiocb *kiocb) return 0; } -/* - * aio_wake_function: - * wait queue callback function for aio notification, - * Simply triggers a retry of the operation via kick_iocb. - * - * This callback is specified in the wait queue entry in - * a kiocb. - * - * Note: - * This routine is executed with the wait queue lock held. - * Since kick_iocb acquires iocb->ctx->ctx_lock, it nests - * the ioctx lock inside the wait queue lock. This is safe - * because this callback isn't used for wait queues which - * are nested inside ioctx lock (i.e. ctx->wait) - */ -static int aio_wake_function(wait_queue_t *wait, unsigned mode, - int sync, void *key) -{ - struct kiocb *iocb = container_of(wait, struct kiocb, ki_wait); - - list_del_init(&wait->task_list); - kick_iocb(iocb); - return 1; -} - static void aio_batch_add(struct address_space *mapping, struct hlist_head *batch_hash) { @@ -1642,8 +1608,6 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb, req->ki_buf = (char __user *)(unsigned long)iocb->aio_buf; req->ki_left = req->ki_nbytes = iocb->aio_nbytes; req->ki_opcode = iocb->aio_lio_opcode; - init_waitqueue_func_entry(&req->ki_wait, aio_wake_function); - INIT_LIST_HEAD(&req->ki_wait.task_list); ret = aio_setup_iocb(req); diff --git a/include/linux/aio.h b/include/linux/aio.h index aea219d7d8d1..811dbb369379 100644 --- a/include/linux/aio.h +++ b/include/linux/aio.h @@ -102,7 +102,6 @@ struct kiocb { } ki_obj; __u64 ki_user_data; /* user's data for completion */ - wait_queue_t ki_wait; loff_t ki_pos; void *private; @@ -140,7 +139,6 @@ struct kiocb { (x)->ki_dtor = NULL; \ (x)->ki_obj.tsk = tsk; \ (x)->ki_user_data = 0; \ - init_wait((&(x)->ki_wait)); \ } while (0) #define AIO_RING_MAGIC 0xa10a10a1 @@ -223,8 +221,6 @@ struct mm_struct; static inline void exit_aio(struct mm_struct *mm) { } #endif /* CONFIG_AIO */ -#define io_wait_to_kiocb(wait) container_of(wait, struct kiocb, ki_wait) - static inline struct kiocb *list_kiocb(struct list_head *h) { return list_entry(h, struct kiocb, ki_list); -- cgit v1.2.3