From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751796Ab2ACEtj (ORCPT ); Mon, 2 Jan 2012 23:49:39 -0500 Received: from mga03.intel.com ([143.182.124.21]:57545 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751091Ab2ACEti (ORCPT ); Mon, 2 Jan 2012 23:49:38 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.71,315,1320652800"; d="scan'208";a="91735927" Date: Tue, 3 Jan 2012 12:49:33 +0800 From: Wu Fengguang To: Chanho Min Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, "'Jens Axboe'" , "'Andrew Morton'" , Rabin Vincent , Linus Walleij Subject: Re: [PATCH] mm/backing-dev.c: fix crash when USB/SCSI device is detached Message-ID: <20120103044933.GA31778@localhost> References: <004401ccc932$444a0070$ccde0150$@min@lge.com> <20120102095711.GA16570@localhost> <002e01ccc9c7$1928c940$4b7a5bc0$@min@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <002e01ccc9c7$1928c940$4b7a5bc0$@min@lge.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 03, 2012 at 12:23:44PM +0900, Chanho Min wrote: > >On Mon, Jan 02, 2012 at 06:38:21PM +0900, ����ȣ wrote: > >> from Chanho Min > >> > >> System may crash in backing-dev.c when removal SCSI device is detached. > >> bdi task is killed by bdi_unregister()/'khubd', but task's point remains. > >> Shortly afterward, If 'wb->wakeup_timer' is expired before > >> del_timer()/bdi_forker_thread, > >> wakeup_timer_fn() may wake up the dead thread which cause the crash. > >> 'bdi->wb.task' should be NULL as this patch. > > > >Is it some race condition between del_timer() and del_timer_sync()? > > > >bdi_unregister() calls > > > > del_timer_sync > > bdi_wb_shutdown > > kthread_stop > > > >in turn, and del_timer_sync() should guarantee wakeup_timer_fn() is > >no longer called to access the stopped task. > > > > It is not race condition. This happens when USB is removed during write-access. > bdi_wakeup_thread_delayed is called after kthread_stop, and timer is activated again. > > bdi_unregister > kthread_stop > bdi_wakeup_thread_delayed (sys_write mostly calls this) > timer fires Ah OK, the timer could be restarted in the mean while, which breaks the synchronization rule in del_timer_sync(). I noticed a related fix is merged recently, does your test kernel contain this commit? commit 7a401a972df8e184b3d1a3fc958c0a4ddee8d312 Author: Rabin Vincent Date: Fri Nov 11 13:29:04 2011 +0100 backing-dev: ensure wakeup_timer is deleted > Anyway,Is this safeguard to prevent from waking up killed thread? This patch makes no guarantee wakeup_timer_fn() will see NULL bdi->wb.task before the task is stopped, so there is still race conditions. And still, the complete fix would be to prevent wakeup_timer_fn() from being called at all. Thanks, Fengguang > >> Signed-off-by: Chanho Min > >> --- > >> mm/backing-dev.c | 1 + > >> 1 files changed, 1 insertions(+), 0 deletions(-) > >> > >> diff --git a/mm/backing-dev.c b/mm/backing-dev.c > >> index 71034f4..4378a5e 100644 > >> --- a/mm/backing-dev.c > >> +++ b/mm/backing-dev.c > >> @@ -607,6 +607,7 @@ static void bdi_wb_shutdown(struct backing_dev_info > >> *bdi) > >> if (bdi->wb.task) { > >> thaw_process(bdi->wb.task); > >> kthread_stop(bdi->wb.task); > >> + bdi->wb.task = NULL; > >> } > >> } > >> > >> -- > >> 1.7.0.4