On Tue, 19 Dec 2006 13:49:18 -0800 "Chen, Kenneth W" <[EMAIL PROTECTED]> wrote:
> Regarding to a bug report on: > http://marc.theaimsgroup.com/?l=linux-kernel&m=116599593200888&w=2 > > flush_workqueue() is not allowed to be called in the softirq context. > However, aio_complete() called from I/O interrupt can potentially call > put_ioctx with last ref count on ioctx and trigger a bug warning. It > is simply incorrect to perform ioctx freeing from aio_complete. > > This patch removes all duplicate ref counting for each kiocb as > reqs_active already used as a request ref count for each active ioctx. > This also ensures that buggy call to flush_workqueue() in softirq > context is eliminated. wait_for_all_aios currently will wait on last > active kiocb. However, it is racy. This patch also tighten it up > by utilizing rcu synchronization mechanism to ensure no further > reference to ioctx before put_ioctx function is run. > hrm, maybe. Does this count as "abuse of the RCU interfaces". Or "reuse"? > > > --- ./fs/aio.c.orig 2006-12-19 08:35:01.000000000 -0800 > +++ ./fs/aio.c 2006-12-19 08:46:34.000000000 -0800 > @@ -308,6 +308,7 @@ static void wait_for_all_aios(struct kio > set_task_state(tsk, TASK_UNINTERRUPTIBLE); > } > __set_task_state(tsk, TASK_RUNNING); > + synchronize_rcu(); > remove_wait_queue(&ctx->wait, &wait); > } argh. Pity the poor fresh-faced wannabe kernel developer who stumbles across a stray synchronize_rcu() in the AIO code and wonders what the hell that's doing there. Please, this needs good commenting. More than zero, anyway. > @@ -425,7 +426,6 @@ static struct kiocb fastcall *__aio_get_ > ring = kmap_atomic(ctx->ring_info.ring_pages[0], KM_USER0); > if (ctx->reqs_active < aio_ring_avail(&ctx->ring_info, ring)) { > list_add(&req->ki_list, &ctx->active_reqs); > - get_ioctx(ctx); > ctx->reqs_active++; > okay = 1; > } > @@ -538,8 +538,6 @@ int fastcall aio_put_req(struct kiocb *r > spin_lock_irq(&ctx->ctx_lock); > ret = __aio_put_req(ctx, req); > spin_unlock_irq(&ctx->ctx_lock); > - if (ret) > - put_ioctx(ctx); > return ret; > } > > @@ -795,8 +793,7 @@ static int __aio_run_iocbs(struct kioctx > */ > iocb->ki_users++; /* grab extra reference */ > aio_run_iocb(iocb); > - if (__aio_put_req(ctx, iocb)) /* drop extra ref */ > - put_ioctx(ctx); > + __aio_put_req(ctx, iocb); > } > if (!list_empty(&ctx->run_list)) > return 1; > @@ -1012,6 +1009,7 @@ int fastcall aio_complete(struct kiocb * > iocb->ki_nbytes - iocb->ki_left, iocb->ki_nbytes); > put_rq: > /* everything turned out well, dispose of the aiocb. */ > + rcu_read_lock(); > ret = __aio_put_req(ctx, iocb); > > spin_unlock_irqrestore(&ctx->ctx_lock, flags); > @@ -1019,9 +1017,7 @@ put_rq: > if (waitqueue_active(&ctx->wait)) > wake_up(&ctx->wait); > > - if (ret) > - put_ioctx(ctx); > - > + rcu_read_unlock(); > return ret; > } > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
