Messages in this thread | | | Date | Wed, 7 Mar 2007 14:14:22 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH 1/3] aio: fix oops because of extra IO control block freeing. |
| |
On Mon, 05 Mar 2007 17:23:33 +0300 Leonid Ananiev <leonid.i.ananiev@linux.intel.com> wrote:
> From Leonid Ananiev > > The patch fixes oops because of extra IO control block freeing. > IO is retried if page could not be invalidated. > > Signed-off-by: Leonid Ananiev <leonid.i.ananiev@intel.com> > > The patch fixes oops "Kernel BUG at fs/aio.c:509" archived at > http://lkml.org/lkml/2007/2/8/337 > The number of IO control block (iocb)users < 0. > If page could not be invalidated by invalidate_inode_pages2_range() > than EIO is returned. It happens if journal_try_to_free_buffers() fails > to drop_buffers(). > This EIO is not differing from real IO competition with EIO and > aio_complete() is called. > Later aio_complete() is called from dio_bio_end_aio() and frees iocb > once more. > > After patch generic_file_direct_IO() sets PgBusy flag in iocb > if page could not be invalidated. iocb is retried after IO competition. > The process is waked up if IO is SYNC else iocb is kicked. > The lines ___if (ret != -EIOCBRETRY)___ is deleted because > nothing set to EIOCBRETRY.
Please copy linux-aio@kvack.org on AIO patches.
> Next patches 2/3 and 3/3 do cleanup only.
I cannot find those patches.
> The patch is applied and tested with aio-stress on 2.6.20 and 2.6.21-rc2 > diff -uprNX linux-2.6.21-rc2/Documentation/dontdiff
Your email client performs space-stuffing, which makes things harder on the receiving end. http://mbligh.org/linuxdocs/Email/Clients/Thunderbird might help.
> linux-2.6.21-rc2/fs/aio.c linux-2.6.21-rc2-aio31/fs/aio.c > --- linux-2.6.21-rc2/fs/aio.c 2007-03-05 00:29:58.000000000 -0800 > +++ linux-2.6.21-rc2-aio31/fs/aio.c 2007-03-05 00:38:11.000000000 -0800 > @@ -723,37 +723,12 @@ static ssize_t aio_run_iocb(struct kiocb > ret = retry(iocb); > current->io_wait = NULL; > > - if (ret != -EIOCBRETRY && ret != -EIOCBQUEUED) { > + if (!kiocbIsPgBusy(iocb) && ret != -EIOCBQUEUED) { > BUG_ON(!list_empty(&iocb->ki_wait.task_list)); > aio_complete(iocb, ret, 0); > } > out: > spin_lock_irq(&ctx->ctx_lock); > - > - if (-EIOCBRETRY == ret) { > - /* > - * OK, now that we are done with this iteration > - * and know that there is more left to go, > - * this is where we let go so that a subsequent > - * "kick" can start the next iteration > - */ > - > - /* will make __queue_kicked_iocb succeed from here on */ > - INIT_LIST_HEAD(&iocb->ki_run_list); > - /* we must queue the next iteration ourselves, if it > - * has already been kicked */ > - if (kiocbIsKicked(iocb)) { > - __queue_kicked_iocb(iocb); > - > - /* > - * __queue_kicked_iocb will always return 1 here, because > - * iocb->ki_run_list is empty at this point so it should > - * be safe to unconditionally queue the context into the > - * work queue. > - */ > - aio_queue_work(ctx); > - } > - } > return ret; > } > > @@ -945,6 +920,10 @@ int fastcall aio_complete(struct kiocb * > wake_up_process(iocb->ki_obj.tsk); > return 1; > } > + if (TestClearPgBusy(iocb)) { // page could not be invalidated > + kick_iocb(iocb); > + return 1; > + } > > info = &ctx->ring_info; > > diff -uprNX linux-2.6.21-rc2/Documentation/dontdiff > linux-2.6.21-rc2/fs/read_write.c linux-2.6.21-rc2-aio31/fs/read_write.c > --- linux-2.6.21-rc2/fs/read_write.c 2007-03-05 00:29:58.000000000 -0800 > +++ linux-2.6.21-rc2-aio31/fs/read_write.c 2007-03-05 04:44:44.000000000 > -0800 > @@ -239,7 +239,7 @@ ssize_t do_sync_read(struct file *filp, > > for (;;) { > ret = filp->f_op->aio_read(&kiocb, &iov, 1, kiocb.ki_pos); > - if (ret != -EIOCBRETRY) > + if (!TestClearPgBusy(&kiocb)) > break; > wait_on_retry_sync_kiocb(&kiocb); > } > @@ -297,7 +297,7 @@ ssize_t do_sync_write(struct file *filp, > > for (;;) { > ret = filp->f_op->aio_write(&kiocb, &iov, 1, kiocb.ki_pos); > - if (ret != -EIOCBRETRY) > + if (!TestClearPgBusy(&kiocb)) > break; > wait_on_retry_sync_kiocb(&kiocb); > } > @@ -463,7 +463,7 @@ ssize_t do_sync_readv_writev(struct file > > for (;;) { > ret = fn(&kiocb, iov, nr_segs, kiocb.ki_pos); > - if (ret != -EIOCBRETRY) > + if (!TestClearPgBusy(&kiocb)) > break; > wait_on_retry_sync_kiocb(&kiocb); > } > diff -uprNX linux-2.6.21-rc2/Documentation/dontdiff > linux-2.6.21-rc2/include/linux/aio.h > linux-2.6.21-rc2-aio31/include/linux/aio.h > --- linux-2.6.21-rc2/include/linux/aio.h 2007-02-04 10:44:54.000000000 -0800 > +++ linux-2.6.21-rc2-aio31/include/linux/aio.h 2007-03-05 > 00:38:11.000000000 -0800 > @@ -34,21 +34,26 @@ struct kioctx; > /* #define KIF_LOCKED 0 */ > #define KIF_KICKED 1 > #define KIF_CANCELLED 2 > +#define KIF_PG_BUSY 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 TestClearPgBusy(iocb) test_and_clear_bit(KIF_PG_BUSY, > &(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 kiocbSetPgBusy(iocb) set_bit(KIF_PG_BUSY, &(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 kiocbClearPgBusy(iocb) clear_bit(KIF_PG_BUSY, &(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 kiocbIsPgBusy(iocb) test_bit(KIF_PG_BUSY, &(iocb)->ki_flags) > > /* is there a better place to document function pointer methods? */ > /** > diff -upr linux-2.6.20/mm/filemap.c linux-2.6.20-aio2/mm/filemap.c > --- linux-2.6.20/mm/filemap.c 2007-02-04 21:44:54.000000000 +0300 > +++ linux-2.6.20-aio2/mm/filemap.c 2007-03-04 21:46:10.000000000 +0300 > @@ -2413,10 +2413,9 @@ generic_file_direct_IO(int rw, struct ki > if (rw == WRITE && mapping->nrpages) { > pgoff_t end = (offset + write_len - 1) > >> PAGE_CACHE_SHIFT; > - int err = invalidate_inode_pages2_range(mapping, > - offset >> PAGE_CACHE_SHIFT, end); > - if (err) > - retval = err; > + if (invalidate_inode_pages2_range(mapping, > + offset >> PAGE_CACHE_SHIFT, end)) > + kiocbSetPgBusy(iocb); > } > } > return retval; > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |