From dee11c2364f51cac53df17d742a0c69097e29a4e Mon Sep 17 00:00:00 2001 From: Ken Chen Date: Sat, 3 Feb 2007 01:13:45 -0800 Subject: [PATCH] aio: fix buggy put_ioctx call in aio_complete - v2 An AIO bug was reported that sleeping function is being called in softirq context: BUG: warning at kernel/mutex.c:132/__mutex_lock_common() Call Trace: [] __mutex_lock_slowpath+0x640/0x6c0 [] mutex_lock+0x20/0x40 [] flush_workqueue+0xb0/0x1a0 [] __put_ioctx+0xc0/0x240 [] aio_complete+0x2f0/0x420 [] finished_one_bio+0x200/0x2a0 [] dio_bio_complete+0x1c0/0x200 [] dio_bio_end_aio+0x60/0x80 [] bio_endio+0x110/0x1c0 [] __end_that_request_first+0x180/0xba0 [] end_that_request_chunk+0x30/0x60 [] scsi_end_request+0x50/0x300 [scsi_mod] [] scsi_io_completion+0x200/0x8a0 [scsi_mod] [] sd_rw_intr+0x330/0x860 [sd_mod] [] scsi_finish_command+0x100/0x1c0 [scsi_mod] [] scsi_softirq_done+0x230/0x300 [scsi_mod] [] blk_done_softirq+0x160/0x1c0 [] __do_softirq+0x200/0x240 [] do_softirq+0x70/0xc0 See report: 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 triggers bug. It is simply incorrect to perform ioctx freeing from aio_complete. The bug is trigger-able from a race between io_destroy() and aio_complete(). A possible scenario: cpu0 cpu1 io_destroy aio_complete wait_for_all_aios { __aio_put_req ... ctx->reqs_active--; if (!ctx->reqs_active) return; } ... put_ioctx(ioctx) put_ioctx(ctx); __put_ioctx bam! Bug trigger! The real problem is that the condition check of ctx->reqs_active in wait_for_all_aios() is incorrect that access to reqs_active is not being properly protected by spin lock. This patch adds that protective spin lock, and at the same time removes all duplicate ref counting for each kiocb as reqs_active is already used as a ref count for each active ioctx. This also ensures that buggy call to flush_workqueue() in softirq context is eliminated. Signed-off-by: "Ken Chen" Cc: Zach Brown Cc: Suparna Bhattacharya Cc: Benjamin LaHaise Cc: Badari Pulavarty Cc: Acked-by: Jeff Moyer Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/aio.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'fs/aio.c') diff --git a/fs/aio.c b/fs/aio.c index ee20fc4240e0..55991e4132a7 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -298,17 +298,23 @@ static void wait_for_all_aios(struct kioctx *ctx) struct task_struct *tsk = current; DECLARE_WAITQUEUE(wait, tsk); + spin_lock_irq(&ctx->ctx_lock); if (!ctx->reqs_active) - return; + goto out; add_wait_queue(&ctx->wait, &wait); set_task_state(tsk, TASK_UNINTERRUPTIBLE); while (ctx->reqs_active) { + spin_unlock_irq(&ctx->ctx_lock); schedule(); set_task_state(tsk, TASK_UNINTERRUPTIBLE); + spin_lock_irq(&ctx->ctx_lock); } __set_task_state(tsk, TASK_RUNNING); remove_wait_queue(&ctx->wait, &wait); + +out: + spin_unlock_irq(&ctx->ctx_lock); } /* wait_on_sync_kiocb: @@ -424,7 +430,6 @@ static struct kiocb fastcall *__aio_get_req(struct kioctx *ctx) 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; } @@ -536,8 +541,6 @@ int fastcall aio_put_req(struct kiocb *req) 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; } @@ -779,8 +782,7 @@ static int __aio_run_iocbs(struct kioctx *ctx) */ 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; @@ -997,14 +999,10 @@ put_rq: /* everything turned out well, dispose of the aiocb. */ ret = __aio_put_req(ctx, iocb); - spin_unlock_irqrestore(&ctx->ctx_lock, flags); - if (waitqueue_active(&ctx->wait)) wake_up(&ctx->wait); - if (ret) - put_ioctx(ctx); - + spin_unlock_irqrestore(&ctx->ctx_lock, flags); return ret; } -- cgit v1.2.3