Pending AIO work/patches
Subsystems (7)
Since AIO development is gaining momentum once again, ocfs2 and samba both appear to be using AIO, NFS needs async semaphores etc, there appears to be an increase in interest in straightening out some of the pending work in this area. So this seems like a good time to re-post some of those patches for discussion and decision.
Just to help sync up, here is an initial list based on the pieces that have been in progress with patches in existence (please feel free to add/update ones I missed or reflected inaccurately here):
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs review
(2) Buffered filesystem AIO read/write (me/Ben)
Status: aio write: Updated to 2.6.12-rc6, needs review
Status: aio read : Needs rework against readahead changes in mainline
(3) POSIX AIO support (Bull: Laurent/Sebastian or Oracle: Joel)
Status: Needs review and discussion ?
(4) AIO for pipes (Chris Mason)
Status: Needs update to latest kernels
(5) Asynchronous semaphore implementation (Ben/Trond?)
Status: Posted - under development & discussion
(6) epoll - AIO integration (Zach Brown/Feng Zhou/wli)
Status: Needs resurrection ?
(7) Vector AIO (aio readv/writev) (Yasushi Saito)
Status: Needs resurrection ?
On my part, I'll start by re-posting (1) for discussion, and then move to (2).
Regards
Suparna
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
må den 20.06.2005 Klokka 17:31 (+0530) skreiv Suparna Bhattacharya:
Quoted 2 lines
(5) Asynchronous semaphore implementation (Ben/Trond?)
Status: Posted - under development & discussion
I'm working on something that attempts to define per-arch low-level primitives (essentially wrappers to the current arch specific code). I expect to post something in the next 2 weeks (or at least before the kernel summit)...
Cheers,
Trond
Quoted 2 lines · Suparna Bhattacharya
(3) POSIX AIO support (Bull: Laurent/Sebastian or Oracle: Joel)
Status: Needs review and discussion ?
I'm currently running some benchmarks (sysbench + MySQL) using AIO, results will be available soon.
I will also release libposix-aio V0.5 at the same time.
(2) will sure help cleanup our code.
Sébastien.
Signature · 10 lines
------------------------------------------------------ Sébastien Dugué BULL/FREC:B1-247 phone: (+33) 476 29 77 70 Bullcom: 229-7770 mailto:[email protected] Linux POSIX AIO: http://www.bullopensource.org/posix ------------------------------------------------------
cover Integrate AIO with wait-bit based filtered wakeups
In reply to Suparna BhattacharyaQuoted 6 lines · Suparna Bhattacharya
Since AIO development is gaining momentum once again, ocfs2 and samba both appear to be using AIO, NFS needs async semaphores etc, there appears to be an increase in interest in straightening out some of the pending work in this area. So this seems like a good time to re-post some of those patches for discussion and decision.
Quoted 6 lines
Just to help sync up, here is an initial list based on the pieces that have been in progress with patches in existence (please feel free to add/update ones I missed or reflected inaccurately here):
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs review
Here is a little bit of background on the motivation behind this set of patches to update AIO for filtered wakeups:
(a) Since the introduction of filtered wakeups support and the wait_bit_queue infrastructure in mainline, it is no longer sufficient to just embed a wait queue entry in the kiocb for AIO operations involving filtered wakeups.
(b) Given that filesystem reads/writes use filtered wakeups underlying wait_on_page_bit, fixing this becomes a pre-req for buffered filesystem AIO.
(c) The wait_bit_queue infrastructure actually enables a cleaner implementation of filesystem AIO because it already provides for an action routine intended to allow both blocking and non-blocking or asynchronous behaviour.
As I was rewriting the patches to address this, there is one other change I made to resolve one remaining ugliness in my earlier patchsets - special casing of the form
if (wait == NULL) wait = &local_wait
to switch to a stack based wait queue entry if not passed a wait queue entry associated with an iocb.
To avoid this, I have tried biting the bullet by including a default wait bit queue entry in the task structure, to be used instead of on-demand allocation of a wait bit queue entry on stack.
All in all, these changes have (hopefully) simplified the code, as well as made it more up-to-date. Comments (including better names etc as requested by Zach) are welcome !
Regards
Suparna
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 1/6] Add a wait queue argument to wait_bit action()
Quoted 16 lines · Suparna Bhattacharya
Here is a little bit of background on the motivation behind this set of patches to update AIO for filtered wakeups:
(a) Since the introduction of filtered wakeups support and the
wait_bit_queueinfrastructure in mainline, it is no longer sufficient to just embed a wait queue entry in the kiocb for AIO operations involving filtered wakeups.
(b) Given that filesystem reads/writes use filtered wakeups underlyingwait_on_page_bit, fixing this becomes a pre-req for buffered filesystem AIO.
(c) Thewait_bit_queueinfrastructure actually enables a cleaner implementation of filesystem AIO because it already provides for an action routine intended to allow both blocking and non-blocking or asynchronous behaviour.Quoted 1 line
Updated to apply to 2.6.12-rc6.
Add a wait queue parameter to the action routine called by __wait_on_bit to allow it to determine whether to block or not.
fs/buffer.c | 2 +- kernel/wait.c | 14 ++++++++------ mm/filemap.c | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-)
kernel/wait.c renamed from kernel/wait.c~modify-wait-bit-action-args +1 -1
@@ -152,14 +152,14 @@ EXPORT_SYMBOL(wake_bit_function);*/ int __sched fastcall__wait_on_bit (wait_queue_head_t *wq,struct wait_bit_queue *q, -int (*action)(void *),unsigned mode) +int (*action)(void *,wait_queue_t *),unsigned mode) {int ret =0 ;
do {
prepare_to_wait(wq, &q->wait, mode);
if (test_bit(q->key.bit_nr, q->key.flags))
- ret = (*action)(q->key.flags);
+ ret = (*action)(q->key.flags, &q->wait);
} while (test_bit(q->key.bit_nr, q->key.flags) && !ret); finish_wait(wq, &q->wait);
return ret;
@@ -167,7 +167,8 @@ EXPORT_SYMBOL(__wait_on_bit);
EXPORT_SYMBOL(__wait_on_bit);
int __sched fastcall out_of_line_wait_on_bit(void *word, int bit,
- int (*action)(void *), unsigned mode)
+ int (*action)(void *, wait_queue_t *),
+ unsigned mode)
{
wait_queue_head_t *wq = bit_waitqueue(word, bit); DEFINE_WAIT_BIT(wait, word, bit);
@@ -178,14 +179,14 @@ EXPORT_SYMBOL(out_of_line_wait_on_bit);
int __sched fastcall __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
- int (*action)(void *), unsigned mode)
+ int (*action)(void *, wait_queue_t *), unsigned mode)
{
int ret = 0;
do {
prepare_to_wait_exclusive(wq, &q->wait, mode); if (test_bit(q->key.bit_nr, q->key.flags)) {
- if ((ret = (*action)(q->key.flags)))
+ if ((ret = (*action)(q->key.flags, &q->wait)))
break;
}
} while (test_and_set_bit(q->key.bit_nr, q->key.flags));
@@ -195,7 +196,8 @@ __wait_on_bit_lock(wait_queue_head_t *wq
EXPORT_SYMBOL(__wait_on_bit_lock);
int __sched fastcall out_of_line_wait_on_bit_lock(void *word, int bit,
- int (*action)(void *), unsigned mode)
+ int (*action)(void *, wait_queue_t *wait),
+ unsigned mode)
{
wait_queue_head_t *wq = bit_waitqueue(word, bit); DEFINE_WAIT_BIT(wait, word, bit);
mm/filemap.c renamed from mm/filemap.c~modify-wait-bit-action-args +1 -1
@@ -133,7 +133,7 @@ void remove_from_page_cache(struct page }EXPORT_SYMBOL (remove_from_page_cache); -static int sync_page (void *word) +static int sync_page (void *word,wait_queue_t *wait) {struct address_space *mapping;struct page *page;
fs/buffer.c renamed from fs/buffer.c~modify-wait-bit-action-args +0 -0
@@ -52,7 +52,7 @@ void wake_up_buffer(struct buffer_head *
bh->b_private = private;
}
-static int sync_buffer(void *word)
+static int sync_buffer(void *word, wait_queue_t *wait)
{
struct block_device *bd;
struct buffer_head *bh
_
include/linux/wait.h +12 -6
@@ -140,11 +151,15 @@ void FASTCALL(__wake_up(wait_queue_head_extern void FASTCALL (__wake_up_locked (wait_queue_head_t *q,unsigned int mode));extern void FASTCALL (__wake_up_sync (wait_queue_head_t *q,unsigned int mode,int nr));void FASTCALL (__wake_up_bit (wait_queue_head_t *,void *,int )); -int FASTCALL (__wait_on_bit (wait_queue_head_t *,struct wait_bit_queue *,int (*)(void *),unsigned )); -int FASTCALL (__wait_on_bit_lock (wait_queue_head_t *,struct wait_bit_queue *,int (*)(void *),unsigned )); +int FASTCALL (__wait_on_bit (wait_queue_head_t *,struct wait_bit_queue *, +int (*)(void *,wait_queue_t *),unsigned )); +int FASTCALL (__wait_on_bit_lock (wait_queue_head_t *,struct wait_bit_queue *, +int (*)(void *,wait_queue_t *),unsigned ));void FASTCALL (wake_up_bit (void *,int )); -int FASTCALL (out_of_line_wait_on_bit (void *,int ,int (*)(void *),unsigned )); -int FASTCALL (out_of_line_wait_on_bit_lock (void *,int ,int (*)(void *),unsigned )); +int FASTCALL (out_of_line_wait_on_bit (void *,int ,int (*)(void *, +wait_queue_t *),unsigned )); +int FASTCALL (out_of_line_wait_on_bit_lock (void *,int ,int (*)(void *, +wait_queue_t *),unsigned ));wait_queue_head_t *FASTCALL (bit_waitqueue (void *,int ));#define wake_up (x) __wake_up(x, TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, 1, NULL)
@@ -364,7 +392,8 @@ int wake_bit_function(wait_queue_t *wait* but has no intention of setting it. */ static inline int wait_on_bit (void *word,int bit, -int (*action)(void *),unsigned mode) +int (*action)(void *,wait_queue_t *), +unsigned mode) {if (!test_bit (bit, word))return 0 ;
@@ -388,7 +417,8 @@ static inline int wait_on_bit(void *word* clear with the intention of setting it, and when done, clearing it. */ static inline int wait_on_bit_lock (void *word,int bit, -int (*action)(void *),unsigned mode) +int (*action)(void *,wait_queue_t *), +unsigned mode) {if (!test_and_set_bit (bit, word))return 0 ;
include/linux/writeback.h +1 -1
@@ -68,7 +68,7 @@ struct writeback_control {*/ void writeback_inodes (struct writeback_control *wbc);void wake_up_inode (struct inode *inode); -int inode_wait (void *); +int inode_wait (void *,wait_queue_t *);void sync_inodes_sb (struct super_block *,int wait);void sync_inodes (int wait);
fs/inode.c +1 -1
@@ -1264,7 +1264,7 @@ void remove_dquot_ref(struct super_block#endif -int inode_wait (void *word) +int inode_wait (void *word,wait_queue_t *wait) {schedule ();return 0 ;
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 2/6] Rename __lock_page to lock_page_slow
In reply to Suparna BhattacharyaIn order to allow for interruptible and asynchronous versions of lock_page in conjunction with the wait_on_bit changes, we need to define low-level lock page routines which take an additional argument, i.e a wait queue entry and may return non-zero status, e.g -EINTR, -EIOCBRETRY, -EWOULDBLOCK etc. This patch renames __lock_page to lock_page_slow, so that __lock_page and __lock_page_slow can denote the versions which take a wait queue parameter.
include/linux/pagemap.h | 4 ++-- mm/filemap.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
include/linux/pagemap.h renamed from include/linux/pagemap.h~lock_page_slow +2 -2
@@ -151,14 +151,14 @@ static inline pgoff_t linear_page_index(return pgoff >> (PAGE_CACHE_SHIFT -PAGE_SHIFT ); } -extern void FASTCALL (__lock_page (struct page *page)); +extern void FASTCALL (lock_page_slow (struct page *page));extern void FASTCALL (unlock_page (struct page *page));static inline void lock_page (struct page *page) {might_sleep ();if (TestSetPageLocked (page)) -__lock_page (page); +lock_page_slow (page); }/*
mm/filemap.c renamed from mm/filemap.c~lock_page_slow +2 -2
@@ -438,14 +438,14 @@ EXPORT_SYMBOL(end_page_writeback);* chances are that on the second loop, the block layer's plug list is empty, * so sync_page() will then return in state TASK_UNINTERRUPTIBLE. */ -void fastcall __lock_page(struct page *page) +void fastcall lock_page_slow(struct page *page) {DEFINE_WAIT_BIT (wait, &page->flags, PG_locked);__wait_on_bit_lock (page_waitqueue (page), &wait, sync_page,TASK_UNINTERRUPTIBLE ); } -EXPORT_SYMBOL (__lock_page); +EXPORT_SYMBOL (lock_page_slow);/* * Note completion of filesystem specific page synchronisation
_
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 2/6] Rename __lock_page to lock_page_slow
I have to whine at least once about obscure names :)
mm/filemap.c EXPORT_SYMBOL(end_page_writeback); · line 448
Quoted 10 lines
mm/filemap.c renamed from mm/filemap.c~lock_page_slow +2
-2-void fastcall __lock_page(struct page *page) +void fastcall lock_page_slow(struct page *page) {DEFINE_WAIT_BIT (wait, &page->flags, PG_locked);__wait_on_bit_lock (page_waitqueue (page), &wait, sync_page,TASK_UNINTERRUPTIBLE ); } -EXPORT_SYMBOL (__lock_page); +EXPORT_SYMBOL (lock_page_slow);
Can we chose a name that describes what it does? Something like lock_page_wait_on_locked()?
- z
Re: [PATCH 2/6] Rename __lock_page to lock_page_slow
Quoted 8 lines · Zach Brown
I have to whine at least once about obscure names :)
Quoted 5 lines
mm/filemap.c renamed from mm/filemap.c~lock_page_slow +1
-1-void fastcall __lock_page(struct page *page) +void fastcall lock_page_slow(struct page *page) {DEFINE_WAIT_BIT (wait, &page->flags, PG_locked);
Quoted 8 lines
mm/filemap.c EXPORT_SYMBOL(end_page_writeback); · line 448
Quoted 5 lines
mm/filemap.c renamed from mm/filemap.c~lock_page_slow +1
-1__wait_on_bit_lock (page_waitqueue (page), &wait, sync_page,TASK_UNINTERRUPTIBLE ); } -EXPORT_SYMBOL (__lock_page); +EXPORT_SYMBOL (lock_page_slow);Can we chose a name that describes what it does? Something like
lock_page_wait_on_locked()?
The usage of the _slow suffix was inspired from fs/buffer.c, __bread_slow, __find_get_block_slow, __getblk_slow etc.
I actually have an earlier version with the name changes you had suggested earlier ... but then wasn't sure if these are really much better, wanted to hear what others had to say. I do not have any particular affinities, will go with whatever is the general consensus.
Regards
Suparna
Quoted 2 lines
- z
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 2/6] Rename __lock_page to lock_page_slow
In reply to Suparna BhattacharyaQuoted 8 lines · Suparna Bhattacharya
In order to allow for interruptible and asynchronous versions of
lock_pagein conjunction with thewait_on_bitchanges, we need to define low-level lock page routines which take an additional argument, i.e a wait queue entry and may return non-zero status, e.g-EINTR,-EIOCBRETRY,-EWOULDBLOCKetc. This patch renames__lock_pagetolock_page_slow, so that__lock_pageand__lock_page_slowcan denote the versions which take a wait queue parameter.
How many users that don't use a waitqueue parameter will be left once all AIO patches go in?
Re: [PATCH 2/6] Rename __lock_page to lock_page_slow
Quoted 10 lines · Christoph Hellwig
Quoted 8 lines · Suparna Bhattacharya
In order to allow for interruptible and asynchronous versions of
lock_pagein conjunction with thewait_on_bitchanges, we need to define low-level lock page routines which take an additional argument, i.e a wait queue entry and may return non-zero status, e.g-EINTR,-EIOCBRETRY,-EWOULDBLOCKetc. This patch renames__lock_pagetolock_page_slow, so that__lock_pageand__lock_page_slowcan denote the versions which take a wait queue parameter.
Quoted 2 lines
How many users that don't use a waitqueue parameter will be left once all AIO patches go in?
Actually looking at the later patches we always seem to pass current->io_wait anyway, so is there a real point for having the argument?
Re: [PATCH 2/6] Rename __lock_page to lock_page_slow
Quoted 11 lines · Christoph Hellwig
Quoted 10 lines · Christoph Hellwig
Quoted 8 lines · Suparna Bhattacharya
In order to allow for interruptible and asynchronous versions of
lock_pagein conjunction with thewait_on_bitchanges, we need to define low-level lock page routines which take an additional argument, i.e a wait queue entry and may return non-zero status, e.g-EINTR,-EIOCBRETRY,-EWOULDBLOCKetc. This patch renames__lock_pagetolock_page_slow, so that__lock_pageand__lock_page_slowcan denote the versions which take a wait queue parameter.
Quoted 2 lines
Quoted 2 lines
How many users that don't use a waitqueue parameter will be left once all AIO patches go in?
Since these patches are intended only for aio reads and (O_SYNC) writes, that still leaves most other users of regular lock_page() as they are.
Quoted 5 lines
Actually looking at the later patches we always seem to pass
current->io_waitanyway, so is there a real point for having the argument?
Having the parameter enables issual of synchronous lock_page() within an AIO path, overriding current->io_wait, (for example, consider the case of a page fault during AIO !), and thus avoids the need to convert and audit more paths to handle -EIOCBRETRY propagation (though it is possible to do so as and when the need arises). This is why I decided to keep this parameter explicit at the low level, and let the caller decide how to invoke it and handle the return value propagation.
Does that make sense ?
BTW, there is also a potential secondary benefit of a low level primitive for asynchronous page IO operations etc directly usable by kernel threads, without having to use the whole gamut of AIO interfaces.
Thanks for reviewing the code !
Regards
Suparna
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 3/6] Interfaces to initialize and to test a wait_bit key
In reply to Suparna BhattacharyaQuoted 3 lines · Suparna Bhattacharya
patches to update AIO for filtered wakeups:
Quoted 1 line
Updated to apply to 2.6.12-rc6
init_wait_bit_key() initializes the key field in an already allocated wait bit structure, useful for async wait bit support. Also separate out the wait bit test to a common routine which can be used by different kinds of wakeup callbacks.
linux-2.6.9-rc1-mm4-suparna/include/linux/wait.h | 17 +++++++++++++++++ linux-2.6.9-rc1-mm4-suparna/kernel/wait.c | 11 ++--------- 2 files changed, 19 insertions(+), 9 deletions(-)
include/linux/wait.h renamed from include/linux/wait.h~init-wait-bit-key +24 -0
@@ -103,6 +103,17 @@ static inline int waitqueue_active(wait_return !list_empty (&q->task_list); } +static inline int test_wait_bit_key (wait_queue_t *wait, +struct wait_bit_key *key) +{ +struct wait_bit_queue *wait_bit + = container_of(wait, struct wait_bit_queue, wait); + +return (wait_bit->key.flags == key->flags && + wait_bit->key.bit_nr == key->bit_nr && + !test_bit (key->bit_nr, key->flags)); +} +/* * Used to distinguish between sync and async io wait context: * sync i/o typically specifies a NULL wait queue entry or a wait
@@ -344,6 +359,19 @@ int wake_bit_function(wait_queue_t *wait (wait)->task = current; \ (wait)->func = autoremove_wake_function; \INIT_LIST_HEAD (&(wait)->task_list); \ + } while (0) + +#define init_wait_bit_key (waitbit, word, bit) \ + do { \ + (waitbit)->key.flags = word; \ + (waitbit)->key.bit_nr = bit; \ + } while (0) + +#define init_wait_bit_task (waitbit, tsk) \ + do { \ + (waitbit)->wait.task = tsk; \ + (waitbit)->wait.func = wake_bit_function; \ + INIT_LIST_HEAD(&(waitbit)->wait.task_list); \ } while (0)/**
kernel/wait.c renamed from kernel/wait.c~init-wait-bit-key +2 -9
@@ -132,16 +132,9 @@ EXPORT_SYMBOL(autoremove_wake_function);int wake_bit_function (wait_queue_t *wait,unsigned mode,int sync,void *arg) { -struct wait_bit_key *key = arg; -struct wait_bit_queue *wait_bit - = container_of(wait, struct wait_bit_queue, wait); - -if (wait_bit->key.flags != key->flags || - wait_bit->key.bit_nr != key->bit_nr || -test_bit (key->bit_nr, key->flags)) +if (!test_wait_bit_key (wait, arg))return 0 ; -else -return autoremove_wake_function (wait, mode, sync, key); +return autoremove_wake_function (wait, mode, sync, arg); }EXPORT_SYMBOL (wake_bit_function);
_
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 4/6] Add default io wait bit field in task struct
In reply to Suparna BhattacharyaQuoted 3 lines · Suparna Bhattacharya
Quoted 2 lines
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs review
Allocates space for the default io wait queue entry (actually a wait bit entry) in the task struct. Doing so simplifies the patches for AIO wait page allowing for cleaner and more efficient implementation, at the cost of 28 additional bytes in task struct vs allocation on demand on-stack.
include/linux/sched.h | 11 +++++++---- kernel/fork.c | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-)
include/linux/sched.h renamed from include/linux/sched.h~tsk-default-io-wait +7 -4
@@ -584,11 +584,14 @@ struct task_struct {unsigned long ptrace_message;siginfo_t *last_siginfo;/* For ptrace use. */ + +/* Space for default IO wait bit entry used for synchronous IO waits */ +struct wait_bit_queue __wait;/* -* current io wait handle: wait queue entry to use for io waits -* If this thread is processing aio, this points at the waitqueue -* inside the currently handled kiocb. It may be NULL (i.e. default -* to a stack based synchronous wait) if its doing sync IO. +* Current IO wait handle: wait queue entry to use for IO waits +* If this thread is processing AIO, this points at the waitqueue +* inside the currently handled kiocb. Otherwise, points to the +* default IO wait field (i.e &__wait.wait above). */ wait_queue_t *io_wait;#ifdef CONFIG_NUMA
kernel/fork.c renamed from kernel/fork.c~tsk-default-io-wait +2 -1
@@ -859,7 +859,8 @@ static task_t *copy_process(unsigned londo_posix_clock_monotonic_gettime (&p->start_time); p->security =NULL ; p->io_context =NULL ; - p->io_wait =NULL ; +init_wait_bit_task (&p->__wait, p); + p->io_wait = &p->__wait.wait; p->audit_context =NULL ;#ifdef CONFIG_NUMA p->mempolicy =mpol_copy (p->mempolicy);
_
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 5/6] AIO wait bit and AIO wake bit for filtered wakeups
In reply to Suparna BhattacharyaQuoted 3 lines · Suparna Bhattacharya
Quoted 2 lines
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs review
Enable wait bit based filtered wakeups to work for AIO. Replaces the wait queue entry in the kiocb with a wait bit structure, to allow enough space for the wait bit key. This adds an extra level of indirection in references to the wait queue entry in the iocb. Also, adds an extra check in aio_wake_function to allow for other kinds of waiters which do not require wait bit, based on the assumption that the key passed in would be NULL in such cases.
fs/aio.c | 20 +++++++++++++------- include/linux/aio.h | 4 ++-- include/linux/wait.h | 18 ++++++++++++++++++ kernel/wait.c | 17 ++++++++++++++--- 4 files changed, 47 insertions(+), 12 deletions(-)
fs/aio.c renamed from fs/aio.c~aio-wait-bit +15 -8
@@ -725,13 +725,13 @@ static ssize_t aio_run_iocb(struct kiocb* the aio_wake_function callback). */ -BUG_ON (current->io_wait !=NULL ); - current->io_wait = &iocb->ki_wait; +BUG_ON (!is_sync_wait (current->io_wait)); + current->io_wait = &iocb->ki_wait.wait; ret =retry (iocb); current->io_wait =NULL ;if (-EIOCBRETRY != ret) {if (-EIOCBQUEUED != ret) { -BUG_ON (!list_empty (&iocb->ki_wait.task_list)); +BUG_ON (!list_empty (&iocb->ki_wait.wait.task_list));aio_complete (iocb, ret,0 );/* must not access the iocb after this */ }
@@ -740,7 +740,7 @@ static ssize_t aio_run_iocb(struct kiocb* Issue an additional retry to avoid waiting forever if * no waits were queued (e.g. in case of a short read). */ -if (list_empty (&iocb->ki_wait.task_list)) +if (list_empty (&iocb->ki_wait.wait.task_list))kiocbSetKicked (iocb); } out:
@@ -886,7 +886,7 @@ void queue_kicked_iocb(struct kiocb *iocunsigned long flags;int run =0 ; -WARN_ON ((!list_empty (&iocb->ki_wait.task_list))); +WARN_ON ((!list_empty (&iocb->ki_wait.wait.task_list)));spin_lock_irqsave (&ctx->ctx_lock, flags); run =__queue_kicked_iocb (iocb);
@@ -1472,7 +1472,13 @@ ssize_t aio_setup_iocb(struct kiocb *kio*/ int aio_wake_function (wait_queue_t *wait,unsigned mode,int sync,void *key) { - struct kiocb *iocb = container_of(wait, struct kiocb, ki_wait); +struct wait_bit_queue *wait_bit + = container_of(wait, struct wait_bit_queue, wait); + struct kiocb *iocb = container_of(wait_bit, struct kiocb, ki_wait); + +/* Assumes that a non-NULL key implies wait bit filtering */ +if (key && !test_wait_bit_key (wait, key)) +return 0 ;list_del_init (&wait->task_list);kick_iocb (iocb);
@@ -1528,8 +1534,9 @@ int fastcall io_submit_one(struct kioctx 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); +init_waitqueue_func_entry (&req->ki_wait.wait, aio_wake_function); +INIT_LIST_HEAD (&req->ki_wait.wait.task_list); + req->ki_run_list.next = req->ki_run_list.prev =NULL ; req->ki_retried =0 ; ret =aio_setup_iocb (req);
kernel/wait.c renamed from kernel/wait.c~aio-wait-bit +2 -1
@@ -132,7 +132,8 @@ EXPORT_SYMBOL(autoremove_wake_function);int wake_bit_function (wait_queue_t *wait,unsigned mode,int sync,void *arg) { -if (!test_wait_bit_key (wait, arg)) +/* Assumes that a non-NULL key implies wait bit filtering */ +if (arg && !test_wait_bit_key (wait, arg))return 0 ;return autoremove_wake_function (wait, mode, sync, key); }
@@ -152,7 +153,12 @@ __wait_on_bit(wait_queue_head_t *wq, strif (test_bit (q->key.bit_nr, q->key.flags)) ret = (*action)(q->key.flags, &q->wait);
} while (test_bit(q->key.bit_nr, q->key.flags) && !ret);
- finish_wait(wq, &q->wait);
+ /*
+ * AIO retries require the wait queue entry to remain queued
+ * for async notification
+ */
+ if (ret != -EIOCBRETRY)
+ finish_wait(wq, &q->wait);
return ret;
}
EXPORT_SYMBOL(__wait_on_bit);
@@ -181,7 +187,12 @@ __wait_on_bit_lock(wait_queue_head_t *wq
break;
}
} while (test_and_set_bit(q->key.bit_nr, q->key.flags));
- finish_wait(wq, &q->wait);
+ /*
+ * AIO retries require the wait queue entry to remain queued
+ * for async notification
+ */
+ if (ret != -EIOCBRETRY)
+ finish_wait(wq, &q->wait);
return ret;
}
EXPORT_SYMBOL(__wait_on_bit_lock);
include/linux/aio.h renamed from include/linux/aio.h~aio-wait-bit +2 -2
@@ -69,7 +69,7 @@ struct kiocb {size_t ki_nbytes;/* copy of iocb->aio_nbytes */ char __user *ki_buf;/* remaining iocb->aio_buf */ size_t ki_left;/* remaining bytes */ -wait_queue_t ki_wait; +struct wait_bit_queue ki_wait;long ki_retried;/* just for testing */ long ki_kicked;/* just for testing */ long ki_queued;/* just for testing */
@@ -90,7 +90,7 @@ struct kiocb { (x)->ki_dtor =NULL ; \ (x)->ki_obj.tsk = tsk; \ (x)->ki_user_data =0 ; \ -init_wait ((&(x)->ki_wait)); \ +init_wait_bit_task ((&(x)->ki_wait), current);\ } while (0)#define AIO_RING_MAGIC 0xa10a10a1
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 6/6] AIO wait page and AIO lock page
In reply to Suparna BhattacharyaQuoted 3 lines · Suparna Bhattacharya
Quoted 2 lines
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs review
Define low-level page wait and lock page routines which take a wait queue entry pointer as an additional parameter and return status (which may be non-zero when the wait queue parameter signifies an asynchronous wait, typically during AIO).
Synchronous IO waits become a special case where the wait queue parameter is the running task's default io wait context. Asynchronous IO waits happen when the wait queue parameter is the io wait context of a kiocb. Code paths which choose to execute synchronous or asynchronous behaviour depending on the called context specify the current io wait context (which points to sync or async context as the case may be) as the wait parameter.
include/linux/pagemap.h | 38 ++++++++++++++------ mm/filemap.c | 27 ++++++++------ 2 files changed, 44 insertions(+), 21 deletions(-)
kernel/sched.c +14 -0
@@ -4428,6 +4428,20 @@ long __sched io_schedule_timeout(long tireturn ret; } +/* +* Sleep only if the wait context passed is not async, +* otherwise return so that a retry can be issued later. +*/ +int __sched io_wait_schedule(wait_queue_t *wait) +{ +if (!is_sync_wait (wait)) +return -EIOCBRETRY ; +io_schedule (); +return 0 ; +} + +EXPORT_SYMBOL (io_wait_schedule); +/** * sys_sched_get_priority_max - return maximum RT priority. * @policy: scheduling class.
mm/filemap.c renamed from mm/filemap.c~aio-wait-page +16 -11
@@ -146,8 +146,7 @@ static int sync_page(void *word, wait_qu mapping =page_mapping (page);if (mapping && mapping->a_ops && mapping->a_ops->sync_page) mapping->a_ops->sync_page (page); -io_schedule (); -return 0 ; +return io_wait_schedule (wait); }/**
@@ -378,13 +377,17 @@ static inline void wake_up_page(struct p__wake_up_bit (page_waitqueue (page), &page->flags, bit); } -void fastcall wait_on_page_bit(struct page *page, int bit_nr) +int fastcall wait_on_page_bit(struct page *page, int bit_nr, +wait_queue_t *wait) { -DEFINE_WAIT_BIT (wait, &page->flags, bit_nr); - -if (test_bit (bit_nr, &page->flags)) -__wait_on_bit (page_waitqueue (page), &wait, sync_page, +if (test_bit (bit_nr, &page->flags)) { +struct wait_bit_queue *wait_bit + = container_of(wait, struct wait_bit_queue, wait); +init_wait_bit_key (wait_bit, &page->flags, bit_nr); +return __wait_on_bit (page_waitqueue (page), wait_bit, sync_page,TASK_UNINTERRUPTIBLE ); + } +return 0 ; }EXPORT_SYMBOL (wait_on_page_bit);
@@ -431,18 +434,20 @@ void end_page_writeback(struct page *pagEXPORT_SYMBOL (end_page_writeback);/* -* Get a lock on the page, assuming we need to sleep to get it. +* Get a lock on the page, assuming we need to wait to get it. * * Ugly: running sync_page() in state TASK_UNINTERRUPTIBLE is scary. If some * random driver's requestfn sets TASK_RUNNING, we could busywait. However * chances are that on the second loop, the block layer's plug list is empty, * so sync_page() will then return in state TASK_UNINTERRUPTIBLE. */ -void fastcall lock_page_slow(struct page *page) +int fastcall lock_page_slow(struct page *page, wait_queue_t *wait) { -DEFINE_WAIT_BIT (wait, &page->flags, PG_locked); +struct wait_bit_queue *wait_bit + = container_of(wait, struct wait_bit_queue, wait); -__wait_on_bit_lock (page_waitqueue (page), &wait, sync_page, +init_wait_bit_key (wait_bit, &page->flags, PG_locked); +return __wait_on_bit_lock (page_waitqueue (page), wait_bit, sync_page,TASK_UNINTERRUPTIBLE ); }EXPORT_SYMBOL (lock_page_slow);
include/linux/pagemap.h renamed from include/linux/pagemap.h~aio-wait-page +21 -8
@@ -151,21 +151,25 @@ static inline pgoff_t linear_page_index(return pgoff >> (PAGE_CACHE_SHIFT -PAGE_SHIFT ); } -extern void FASTCALL (lock_page_slow (struct page *page)); +extern int FASTCALL (lock_page_slow (struct page *page,wait_queue_t *wait));extern void FASTCALL (unlock_page (struct page *page)); -static inline void lock_page (struct page *page) +static inline int __lock_page (struct page *page,wait_queue_t *wait) {might_sleep ();if (TestSetPageLocked (page)) -lock_page_slow (page); +return lock_page_slow (page, wait); +return 0 ; } + +#define lock_page (page) __lock_page(page, ¤t->__wait.wait)/* * This is exported only for wait_on_page_locked/wait_on_page_writeback. * Never use this directly! */ -extern void FASTCALL (wait_on_page_bit (struct page *page,int bit_nr)); +extern int FASTCALL (wait_on_page_bit (struct page *page,int bit_nr, +wait_queue_t *wait));/* * Wait for a page to be unlocked.
@@ -174,20 +178,29 @@ extern void FASTCALL(wait_on_page_bit(st* ie with increased "page->count" so that the page won't * go away during the wait.. */ -static inline void wait_on_page_locked (struct page *page) +static inline int __wait_on_page_locked (struct page *page,wait_queue_t *wait) {if (PageLocked (page)) -wait_on_page_bit (page, PG_locked); +return wait_on_page_bit (page, PG_locked, wait); +return 0 ; } +#define wait_on_page_locked (page) \ + __wait_on_page_locked(page, ¤t->__wait.wait) +/* * Wait for a page to complete writeback */ -static inline void wait_on_page_writeback (struct page *page) +static inline int __wait_on_page_writeback (struct page *page, +wait_queue_t *wait) {if (PageWriteback (page)) -wait_on_page_bit (page, PG_writeback); +return wait_on_page_bit (page, PG_writeback, wait); +return 0 ; } +#define wait_on_page_writeback (page) \ + __wait_on_page_writeback(page, ¤t->__wait.wait) +extern void end_page_writeback (struct page *page);
/*
* Fault a userspace page into pagetables. Return non-zero on a fault.
include/linux/sched.h +1 -0
@@ -165,6 +165,7 @@ extern void show_stack(struct task_strucvoid io_schedule (void );long io_schedule_timeout (long timeout); +int io_wait_schedule (wait_queue_t *wait);extern void cpu_init (void );extern void trap_init (void );
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 0/6] Integrate AIO with wait-bit based filtered wakeups
In reply to Suparna BhattacharyaQuoted 40 lines · Suparna Bhattacharya
Quoted 6 lines · Suparna Bhattacharya
Since AIO development is gaining momentum once again, ocfs2 and samba both appear to be using AIO, NFS needs async semaphores etc, there appears to be an increase in interest in straightening out some of the pending work in this area. So this seems like a good time to re-post some of those patches for discussion and decision.
Quoted 6 lines
Just to help sync up, here is an initial list based on the pieces that have been in progress with patches in existence (please feel free to add/update ones I missed or reflected inaccurately here):
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs reviewHere is a little bit of background on the motivation behind this set of patches to update AIO for filtered wakeups:
(a) Since the introduction of filtered wakeups support and the
wait_bit_queueinfrastructure in mainline, it is no longer sufficient to just embed a wait queue entry in the kiocb for AIO operations involving filtered wakeups.
(b) Given that filesystem reads/writes use filtered wakeups underlyingwait_on_page_bit, fixing this becomes a pre-req for buffered filesystem AIO.
(c) Thewait_bit_queueinfrastructure actually enables a cleaner implementation of filesystem AIO because it already provides for an action routine intended to allow both blocking and non-blocking or asynchronous behaviour.As I was rewriting the patches to address this, there is one other change I made to resolve one remaining ugliness in my earlier patchsets - special casing of the form
if (wait == NULL) wait = &local_waitto switch to a stack based wait queue entry if not passed a wait queue entry associated with an iocb.
To avoid this, I have tried biting the bullet by including a default wait bit queue entry in the task structure, to be used instead of on-demand allocation of a wait bit queue entry on stack.
Quoted 7 lines
All in all, these changes have (hopefully) simplified the code, as well as made it more up-to-date. Comments (including better names etc as requested by Zach) are welcome !
Regards
Suparna
Just found a bug in aio_run_iocb: after having called the retry
method for the iocb, current->io_wait is RESET to NULL. While this does not affect applications doing only AIO, applications mixing sync and async IO (MySQL for example) end up crashing later on in the sync path when calling lock_page_slow as the io_wait queue is NULL.
Therefore after the retry method has been called the task io_wait
queue should be set to the default queue.
This patch applies over Suparna's wait-bit patchset and maybe should
be folded into aio-wait-bit.
Sébastien.
Signature · 10 lines
------------------------------------------------------ Sébastien Dugué BULL/FREC:B1-247 phone: (+33) 476 29 77 70 Bullcom: 229-7770 mailto:[email protected] Linux POSIX AIO: http://www.bullopensource.org/posix ------------------------------------------------------
Re: [PATCH 0/6] Integrate AIO with wait-bit based filtered wakeups
Quoted 49 lines · Sébastien Dugué
Quoted 40 lines · Suparna Bhattacharya
Quoted 6 lines · Suparna Bhattacharya
Since AIO development is gaining momentum once again, ocfs2 and samba both appear to be using AIO, NFS needs async semaphores etc, there appears to be an increase in interest in straightening out some of the pending work in this area. So this seems like a good time to re-post some of those patches for discussion and decision.
Quoted 6 lines
Just to help sync up, here is an initial list based on the pieces that have been in progress with patches in existence (please feel free to add/update ones I missed or reflected inaccurately here):
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs reviewHere is a little bit of background on the motivation behind this set of patches to update AIO for filtered wakeups:
(a) Since the introduction of filtered wakeups support and the
wait_bit_queueinfrastructure in mainline, it is no longer sufficient to just embed a wait queue entry in the kiocb for AIO operations involving filtered wakeups.
(b) Given that filesystem reads/writes use filtered wakeups underlyingwait_on_page_bit, fixing this becomes a pre-req for buffered filesystem AIO.
(c) Thewait_bit_queueinfrastructure actually enables a cleaner implementation of filesystem AIO because it already provides for an action routine intended to allow both blocking and non-blocking or asynchronous behaviour.As I was rewriting the patches to address this, there is one other change I made to resolve one remaining ugliness in my earlier patchsets - special casing of the form
if (wait == NULL) wait = &local_waitto switch to a stack based wait queue entry if not passed a wait queue entry associated with an iocb.
To avoid this, I have tried biting the bullet by including a default wait bit queue entry in the task structure, to be used instead of on-demand allocation of a wait bit queue entry on stack.
Quoted 7 lines
All in all, these changes have (hopefully) simplified the code, as well as made it more up-to-date. Comments (including better names etc as requested by Zach) are welcome !
Regards
Suparna
Quoted 6 lines
Just found a bug in
aio_run_iocb: after having called the retry
method for the iocb,current->io_waitis RESET to NULL. While this does not affect applications doing only AIO, applications mixing sync and async IO (MySQL for example) end up crashing later on in the sync path when callinglock_page_slowas theio_waitqueue is NULL.
Yes this is a problem. I had spotted it too but the implications hadn't registered well enough for prompt fix - thanks for the patch.
Regards
Suparna
Quoted 20 lines
Therefore after the retry method has been called the task
io_wait
queue should be set to the default queue.This patch applies over Suparna's wait-bit patchset and maybe should
be folded into aio-wait-bit.Sébastien.
Signature · 10 lines
------------------------------------------------------ Sébastien Dugué BULL/FREC:B1-247 phone: (+33) 476 29 77 70 Bullcom: 229-7770 mailto:[email protected] Linux POSIX AIO: http://www.bullopensource.org/posix ------------------------------------------------------
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Benjamin LaHaise
maintainerQuoted 5 lines · Suparna Bhattacharya
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs review
(2) Buffered filesystem AIO read/write (me/Ben)
Status: aio write: Updated to 2.6.12-rc6, needs review
Status: aio read : Needs rework against readahead changes in mainline
I've looked over the patches from today and they seem quite sane. Comments pending...
Quoted 2 lines
(3) POSIX AIO support (Bull: Laurent/Sebastian or Oracle: Joel)
Status: Needs review and discussion ?
The latest version incorporates changes from the last round of feedback (great work Sebastien!) and updates the library license, so people should definately take a closer look. This includes the necessary changes for in-kernel signal support, as well as minimal conversion done on iocbs (the layout matches the in-kernel iocb).
A quick reading shows that most of it looks quite good. I have to stare at the cancellation code a bit more closely, though.
Quoted 2 lines
(4) AIO for pipes (Chris Mason)
Status: Needs update to latest kernels
This likely needs a rewrite with whatever the final semaphore operations turn out to look like, as it gets very easy to convert down() into aio_down() and that minimises the changes to the code.
Quoted 2 lines
(5) Asynchronous semaphore implementation (Ben/Trond?)
Status: Posted - under development & discussion
I got the aio_down() variant working with cancellation now, and should be able to post an updated series of patches against 2.6.12 shortly.
Quoted 2 lines
(6) epoll - AIO integration (Zach Brown/Feng Zhou/wli)
Status: Needs resurrection ?
What are folks thoughts in this area? Zach's patches took the approach of making multishot iocbs possible, which helped avoid the overhead of plain aio_poll's command setup, which was quite visible in pipetest. Zach -- did you do any benchmarking on your aio-epoll patches?
Quoted 2 lines
(7) Vector AIO (aio readv/writev) (Yasushi Saito)
Status: Needs resurrection ?
Zach also made some noises about this recently...
-ben
Signature · 1 line
"Time is what keeps everything from happening all at once." -- John Wheeler
Quoted 9 lines
Quoted 3 lines
(6) epoll - AIO integration (Zach Brown/Feng Zhou/wli)
Status: Needs resurrection ?What are folks thoughts in this area? Zach's patches took the approach of making multishot iocbs possible, which helped avoid the overhead of plain aio_poll's command setup, which was quite visible in pipetest. Zach -- did you do any benchmarking on your aio-epoll patches?
No, what little work I did on this was just pushing for stable functionality. I had a little test app that was still missing event delivery occasionally. I'm sure it'd be easy enough to track down. It still seems like a pretty reasonable translation of epoll event delivery through the aio completion queue. I'm not thrilled with the epoll edge delivery semantics, though, it would be nice to make duplicate event generation contigent on servicing an initial event. EPOLLIN being throttled until read activity is seen on the fd, that kind of thing. Nontrivial work, of course.
Quoted 4 lines
Quoted 2 lines
(7) Vector AIO (aio readv/writev) (Yasushi Saito)
Status: Needs resurrection ?Zach also made some noises about this recently...
Yeah, I've got a patch working that adds CMD_AIO_P{WRITE,READ}V for ext3 via some aio->aio_p{read,werive}v ops. It's currently against some distro 2.6, but I'll port it up to current and post the patch. It seems pretty noncontroversial -- one obviously wants to scatter/gather file-contiguous IO with tiny iovec elements, which bubble down well to the generic fs/block helpers, rather than trying to get the various layers to merge many large iocb submissions that can be found to be file-contiguous.
- z
Quoted 12 lines · Benjamin LaHaise
Quoted 2 lines
(3) POSIX AIO support (Bull: Laurent/Sebastian or Oracle: Joel)
Status: Needs review and discussion ?The latest version incorporates changes from the last round of feedback (great work Sebastien!) and updates the library license, so people should definately take a closer look. This includes the necessary changes for in-kernel signal support, as well as minimal conversion done on iocbs (the layout matches the in-kernel iocb).
A quick reading shows that most of it looks quite good. I have to stare at the cancellation code a bit more closely, though.
As I understand it, cancellation needs support from the
underlying filesystem in order to dequeue requests no yet commited to disk.
So far there is no support from the kernel aside from the USB gadget
driver which registers its own 'iocb->ki_cancel' method for dequeuing requests.
Sébastien.
Signature · 10 lines
------------------------------------------------------ Sébastien Dugué BULL/FREC:B1-247 phone: (+33) 476 29 77 70 Bullcom: 229-7770 mailto:[email protected] Linux POSIX AIO: http://www.bullopensource.org/posix ------------------------------------------------------
Quoted 8 lines · Suparna Bhattacharya
Since AIO development is gaining momentum once again, ocfs2 and samba both appear to be using AIO, NFS needs async semaphores etc, there appears to be an increase in interest in straightening out some of the pending work in this area. So this seems like a good time to re-post some of those patches for discussion and decision. Just to help sync up, here is an initial list based on the pieces that have been in progress with patches in existence (please feel free to add/update ones I missed or reflected inaccurately here):
I'm going to keep going over these until I get tired of it for general bulletproofing purposes, but all indications thus far are good.
-- wli
cover Buffered filesystem AIO read/write
In reply to Suparna BhattacharyaQuoted 10 lines · Suparna Bhattacharya
Since AIO development is gaining momentum once again, ocfs2 and samba both appear to be using AIO, NFS needs async semaphores etc, there appears to be an increase in interest in straightening out some of the pending work in this area. So this seems like a good time to re-post some of those patches for discussion and decision.
Just to help sync up, here is an initial list based on the pieces that have been in progress with patches in existence (please feel free to add/update ones I missed or reflected inaccurately here):
Quoted 10 lines
(1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
Status: Updated to 2.6.12-rc6, needs review
(2) Buffered filesystem AIO read/write (me/Ben)
Status: aio write: Updated to 2.6.12-rc6, needs review
Status: aio read : Needs rework against readahead changes in mainline
...
...
On my part, I'll start by re-posting (1) for discussion, and then move to (2).
Feedback on (1) seems positive so far, so now moving to (2), here are the patches that implement the changes to make filesystem AIO read and write truly asynchronous even without O_DIRECT. With these patches in place it will no longer be necessary for the POSIX AIO library (from Sébastien et al) to force O_DIRECT and memcpy for alignment. (Samba should find this useful)
There are 2 patches: one for buffered filesystem AIO read and the other for buffered filesystem AIO O_SYNC write. These build on the AIO wait bit integration patches posted earlier.
Comments would be appreciated.
The full patchset including (1) and (2) above is available at www.kernel.org/pub/linux/kernel/people/suparna/aio/2612
Regards
Suparna
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
PATCH 1/2 Filesystem AIO read
Quoted 1 line · Suparna Bhattacharya
Quoted 1 line
(2) Buffered filesystem AIO read/write (me/Ben)
Filesystem aio read:
Converts the wait for page to become uptodate (lock page) after readahead/readpage (in do_generic_mapping_read) to a retry exit, to make buffered filesystem AIO reads actually synchronous.
The patch avoids exclusive wakeups with AIO, a problem originally spotted by Chris Mason, though the reasoning for why it is an issue is now much clearer (see explanation in the comment below in aio.c), and the solution is perhaps slightly simpler.
fs/aio.c | 11 ++++++++++- mm/filemap.c | 20 +++++++++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-)
mm/filemap.c +17 -3
@@ -770,6 +770,11 @@if (!isize) goto out; +if (in_aio ()) { +/* Avoid repeat readahead */ +if (is_retried_kiocb (io_wait_to_kiocb (current->io_wait))) + next_index = last_index; + } end_index = (isize -1 ) >>PAGE_CACHE_SHIFT ;for (;;) {struct page *page;
@@ -771,7 +771,11 @@ page_ok: page_not_up_to_date:/* Get exclusive access to the page ... */ -lock_page (page); + +if ((error =__lock_page (page, current->io_wait))) { +pr_debug ("queued lock page \n" ); + goto readpage_error; + }/* Did it get unhashed before we got the lock? */ if (!page->mapping) {
@@ -793,7 +798,8 @@ readpage: goto readpage_error;if (!PageUptodate (page)) { -lock_page (page); +if ((error =__lock_page (page, current->io_wait))) + goto readpage_error;if (!PageUptodate (page)) {if (page->mapping ==NULL ) {/*
@@ -880,7 +880,11 @@ readpage: goto page_ok; readpage_error: -/* UHHUH! A synchronous read error occurred. Report it */ +/* We don't have uptodate data in the page yet */ +/* Could be due to an error or because we need to +* retry when we get an async i/o notification. +* Report the reason. +*/ desc->error = error;page_cache_release (page); goto out;
fs/aio.c +10 -1
@@ -1473,7 +1473,16 @@list_del_init (&wait->task_list);kick_iocb (iocb); -return 1 ; +/* +* Avoid exclusive wakeups with retries since an exclusive wakeup +* may involve implicit expectations of waking up the next waiter +* and there is no guarantee that the retry will take a path that +* would do so. For example if a page has become up-to-date, then +* a retried read may end up straightaway performing a copyout +* and not go through a lock_page - unlock_page that would have +* passed the baton to the next waiter. +*/ +return 0 ; } int fastcall io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
PATCH 2/2 Filesystem AIO write
In reply to Suparna BhattacharyaQuoted 2 lines · Suparna Bhattacharya
Quoted 1 line · Suparna Bhattacharya
(2) Buffered filesystem AIO read/write (me/Ben)
Filesystem AIO write
AIO support for O_SYNC buffered writes, built over O_SYNC-speedup. It uses the tagged radix tree lookups to writeout just the pages pertaining to this request, and retries instead of blocking for writeback to complete on the same range. All the writeout is issued at the time of io submission, and there is a check to make sure that retries skip over straight to the wait_on_page_writeback_range.
include/linux/aio.h | 8 ++++- mm/filemap.c | 82 +++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 21 deletions(-)
include/linux/aio.h +7 -1
@@ -27,21 +27,26 @@ struct kioctx;#define KIF_LOCKED 0#define KIF_KICKED 1#define KIF_CANCELLED 2 +#define KIF_SYNCED 3#define kiocbTryLock (iocb) test_and_set_bit(KIF_LOCKED, &(iocb)->ki_flags)#define kiocbTryKick (iocb) test_and_set_bit(KIF_KICKED, &(iocb)->ki_flags) +#define kiocbTrySync (iocb) test_and_set_bit(KIF_SYNCED, &(iocb)->ki_flags)#define kiocbSetLocked (iocb) set_bit(KIF_LOCKED, &(iocb)->ki_flags)#define kiocbSetKicked (iocb) set_bit(KIF_KICKED, &(iocb)->ki_flags)#define kiocbSetCancelled (iocb) set_bit(KIF_CANCELLED, &(iocb)->ki_flags) +#define kiocbSetSynced (iocb) set_bit(KIF_SYNCED, &(iocb)->ki_flags)#define kiocbClearLocked (iocb) clear_bit(KIF_LOCKED, &(iocb)->ki_flags)#define kiocbClearKicked (iocb) clear_bit(KIF_KICKED, &(iocb)->ki_flags)#define kiocbClearCancelled (iocb) clear_bit(KIF_CANCELLED, &(iocb)->ki_flags) +#define kiocbClearSynced (iocb) clear_bit(KIF_SYNCED, &(iocb)->ki_flags)#define kiocbIsLocked (iocb) test_bit(KIF_LOCKED, &(iocb)->ki_flags)#define kiocbIsKicked (iocb) test_bit(KIF_KICKED, &(iocb)->ki_flags)#define kiocbIsCancelled (iocb) test_bit(KIF_CANCELLED, &(iocb)->ki_flags) +#define kiocbIsSynced (iocb) test_bit(KIF_SYNCED, &(iocb)->ki_flags)struct kiocb {struct list_head ki_run_list;
@@ -184,7 +189,8 @@ do { \ } \ } while (0) -#define io_wait_to_kiocb (wait) container_of(wait, struct kiocb, ki_wait) +#define io_wait_to_kiocb (io_wait) container_of(container_of(io_wait, \ + struct wait_bit_queue, wait), struct kiocb, ki_wait)#define is_retried_kiocb (iocb) ((iocb)->ki_retried > 1)#include <linux/aio_abi.h>
mm/filemap.c +57 -18
153 lines not on this page · raw message
ret = __generic_file_aio_write_nolock(iocb, &local_iov, 1,
&iocb->ki_pos);
up(&inode->i_sem);
+osync:
if (ret > 0 && ((file->f_flags & O_SYNC) || IS_SYNC(inode))) {
ssize_t err;
err = sync_page_range(inode, mapping, pos, ret);
- if (err < 0)
+ if (err < 0) {
ret = err;
+ iocb->ki_pos = pos;
+ }
}
return ret;
}
Signature · 3 lines
Suparna Bhattacharya ([email protected]) Linux Technology Center IBM Software Lab, India
Re: [PATCH 0/2] Buffered filesystem AIO read/write
In reply to Suparna BhattacharyaQuoted 7 lines · Suparna Bhattacharya
Feedback on (1) seems positive so far, so now moving to (2), here are the patches that implement the changes to make filesystem AIO read and write truly asynchronous even without
O_DIRECT. With these patches in place it will no longer be necessary for the POSIX AIO library (from Sébastien et al) to forceO_DIRECTand memcpy for alignment. (Samba should find this useful)
Wonderful ! That's exactly what we need - thanks. I could have fixed this in userspace but it would be rather ugly.
Jeremy.