[lkml]   [2007]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] aio: fix kernel bug when page is temporally busy

On Feb 9, 2007, at 6:05 AM, Suparna Bhattacharya wrote:

> On Fri, Feb 09, 2007 at 11:40:27AM +0100, Jiri Kosina wrote:
>> On Fri, 9 Feb 2007, Andrew Morton wrote:
>>>> @@ -1204,7 +1204,7 @@ generic_file_aio_read(struct kiocb *iocb,
>>>> const struct iovec *iov,
>>>> do_generic_file_read(filp,ppos,&desc,file_read_actor);
>>>> retval += desc.written;
>>>> if (desc.error) {
>>>> - retval = retval ?: desc.error;
>>>> + retval = desc.error;

I was worried about this too.

> blocking. The high level AIO code (see aio_rw_vect_rety) has the
> ability
> to handle this.

I had missed this, and yeah, that's some level of comfort.

But I'm not convinced we can guarantee that's safe. The positive
return code that aio_rw_vect_retry() sees is telling it that some IO
has completed and, arguably, that no more IO is in flight. If we
return partial progress from generic_file_aio_read() while we have an
iocb in a wait queue then we are adding yet another invariant. That
while an iocb is pending from a previous call down the call chain, we
can't return a non-aio negative error. Doing so would cause fs/aio.c
to complete while there is still an iocb on a wait queue from a
previous retry attempt. Right?

I also noticed something just now while poking around these paths to
see if I could get the start of generic_file_aio_read() to fail when
it had previously succeeded. What's to stop another task from racing
to set O_DIRECT between retries?

That sounds like a pretty hilarious way to get a read retry to fail
due to buffer misalignment while a previously buffered instance of it
is still in flight. Hi-larious.

In thinking about this a discussing it with Chris a bit, I wonder if
the right fix isn't to refuse changing O_DIRECT via setfl() once any
IO paths have started on the filp. Something like:

filp->frozen_flags |= O_DIRECT

at the start of paths and check it in setfl()?

Are we similarly worried about O_APPEND?

- z
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2007-02-09 18:07    [W:0.059 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site