Attachment 144812 Details for Bug 190729 – aio_complete should not drop the last reference to an ioctx

[patch] aio_complete should not drop the last reference to an ioctx

linux-2.6-fix-buggy-put_ioctx-call-in-aio_complete-v2.patch (text/plain), 6.90 KB, created by Jeff Moyer on 2007-01-04 15:53:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Moyer

Created: 2007-01-04 15:53:41 UTC

Size: 6.90 KB

patch

obsolete

>Return-Path: <owner-linux-aio@kvack.org>
>Received: from mail.boston.redhat.com ([unix socket])
>	by mail.boston.redhat.com (Cyrus v2.1.12) with LMTP; Fri, 29 Dec 2006 20:35:54 -0500
>X-Sieve: CMU Sieve 2.2
>Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26])
>	by mail.boston.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id kBU1ZrTQ017771
>	for <jmoyer@boston.redhat.com>; Fri, 29 Dec 2006 20:35:53 -0500
>Received: from mx2.redhat.com (mx2.redhat.com [10.255.15.25])
>	by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id kBU1ZqBX003444;
>	Fri, 29 Dec 2006 20:35:52 -0500
>Received: from kanga.kvack.org (kanga.kvack.org [66.96.29.28])
>	by mx2.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id kBU1ZpdO029988;
>	Fri, 29 Dec 2006 20:35:51 -0500
>Received: (root@kanga.kvack.org) by kvack.org id <S26598AbWL3Be5>;
>	Fri, 29 Dec 2006 20:34:57 -0500
>Received: (linux-aio@kanga.kvack.org) by kvack.org id <S26600AbWL3Beq>;
>	Fri, 29 Dec 2006 20:34:46 -0500
>Received: from mga03.intel.com ([143.182.124.21]:31322 "EHLO mga03.intel.com")
>	by kvack.org with ESMTP id <S26598AbWL3Bee>;
>	Fri, 29 Dec 2006 20:34:34 -0500
>Received: from azsmga001.ch.intel.com ([10.2.17.19])
>  by mga03.intel.com with ESMTP; 29 Dec 2006 17:34:26 -0800
>Received: from kwchen-mobl1.amr.corp.intel.com (HELO kwchenmobl1) ([10.3.52.214])
>  by azsmga001.ch.intel.com with ESMTP; 29 Dec 2006 17:34:26 -0800
>X-ExtLoop1: 1
>X-IronPort-AV: i="4.12,220,1165219200"; 
>   d="scan'208"; a="163599566:sNHT35466557"
>From: "Chen, Kenneth W" <kenneth.w.chen@intel.com>
>To: "'Andrew Morton'" <akpm@osdl.org>, "'Benjamin LaHaise'" <bcrl@kvack.org>,
>        <zach.brown@oracle.com>
>Cc: <linux-aio@kvack.org>, <linux-kernel@vger.kernel.org>
>Subject: [patch] aio: fix buggy put_ioctx call in aio_complete - v2
>Date: 	Fri, 29 Dec 2006 17:34:26 -0800
>Message-ID: <000101c72bb2$a47bfa70$d634030a@amr.corp.intel.com>
>MIME-Version: 1.0
>Content-Type: text/plain;
>	charset="us-ascii"
>Content-Transfer-Encoding: 7bit
>X-Mailer: Microsoft Office Outlook 11
>Thread-Index: AccrsqREOG8bzD1gR3q5CgACkLhnew==
>X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2180
>X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.0.3
>Original-Recipient: rfc822;list-linux-aio@kvack.org
>Sender: owner-linux-aio@kvack.org
>Precedence: bulk
>X-Loop: 	owner-majordomo@kvack.org
>X-RedHat-Spam-Score: 0.001 
>
>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:
>     [<a000000100577b00>] __mutex_lock_slowpath+0x640/0x6c0
>     [<a000000100577ba0>] mutex_lock+0x20/0x40
>     [<a0000001000a25b0>] flush_workqueue+0xb0/0x1a0
>     [<a00000010018c0c0>] __put_ioctx+0xc0/0x240
>     [<a00000010018d470>] aio_complete+0x2f0/0x420
>     [<a00000010019cc80>] finished_one_bio+0x200/0x2a0
>     [<a00000010019d1c0>] dio_bio_complete+0x1c0/0x200
>     [<a00000010019d260>] dio_bio_end_aio+0x60/0x80
>     [<a00000010014acd0>] bio_endio+0x110/0x1c0
>     [<a0000001002770e0>] __end_that_request_first+0x180/0xba0
>     [<a000000100277b90>] end_that_request_chunk+0x30/0x60
>     [<a0000002073c0c70>] scsi_end_request+0x50/0x300 [scsi_mod]
>     [<a0000002073c1240>] scsi_io_completion+0x200/0x8a0 [scsi_mod]
>     [<a0000002074729b0>] sd_rw_intr+0x330/0x860 [sd_mod]
>     [<a0000002073b3ac0>] scsi_finish_command+0x100/0x1c0 [scsi_mod]
>     [<a0000002073c2910>] scsi_softirq_done+0x230/0x300 [scsi_mod]
>     [<a000000100277d20>] blk_done_softirq+0x160/0x1c0
>     [<a000000100083e00>] __do_softirq+0x200/0x240
>     [<a000000100083eb0>] 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 <kenneth.w.chen@intel.com>
>
>--- ./fs/aio.c.orig	2006-12-21 08:08:14.000000000 -0800
>+++ ./fs/aio.c	2006-12-21 08:14:27.000000000 -0800
>@@ -298,17 +298,23 @@ static void wait_for_all_aios(struct kio
> 	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_
> 	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 *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;
> }
> 
>@@ -782,8 +785,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;
>@@ -998,14 +1000,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;
> }
> 
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-aio' in
>the body to majordomo@kvack.org.  For more info on Linux AIO,
>see: http://www.kvack.org/aio/
>Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
>

Actions: View | Diff

Attachments on bug 190729: 144812