From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753154Ab1JaFbO (ORCPT ); Mon, 31 Oct 2011 01:31:14 -0400 Received: from mga03.intel.com ([143.182.124.21]:65343 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752742Ab1JaFbN (ORCPT ); Mon, 31 Oct 2011 01:31:13 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.69,429,1315206000"; d="scan'208";a="68685010" Subject: Re: Multi-partition block layer behaviour From: Shaohua Li To: Tiju Jacob Cc: "linux-kernel@vger.kernel.org" In-Reply-To: References: <1319676161.22361.148.camel@sli10-conroe> Content-Type: text/plain; charset="UTF-8" Date: Mon, 31 Oct 2011 13:39:24 +0800 Message-ID: <1320039564.22361.178.camel@sli10-conroe> Mime-Version: 1.0 X-Mailer: Evolution 2.32.2 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2011-10-31 at 12:35 +0800, Tiju Jacob wrote: > On Thu, Oct 27, 2011 at 6:12 AM, Shaohua Li wrote: > > On Wed, 2011-10-26 at 18:10 +0800, Tiju Jacob wrote: > >> >> 1. When an I/O request is made to the filesystem, process 'A' acquires > >> >> a mutex FS lock and a mutex block driver lock. > >> >> > >> >> 2. Process 'B' tries to acquire the mutex FS lock, which is not > >> >> available. Hence, it goes to sleep. Due to the new plugging mechanism, > >> >> before going to sleep, shcedule() is invoked which disables preemption > >> >> and the context becomes atomic. In schedule(), the newly added > >> >> blk_flush_plug_list() is invoked which unplugs the block driver. > >> >> > >> >> 3) During unplug operation the block driver tries to acquire the mutex > >> >> lock which fails, because the lock was held by process 'A'. Previous > >> >> invocation of scheudle() in step 2 has already made the context as > >> >> atomic, hence the error "Schedule while atomic" occured. > >> > if blk_flush_plug_list() is called in schedule(), it will use > >> > blk_run_queue_async > >> > to unplug the queue. This runs in a workqueue. So how could this happen? > >> > > >> > >> The call stack goes as follows: > >> > >> From schedule() it calls blk_schedule_flush_plug() and > >> blk_flush_plug_list() gets invoked. > >> > >> In blk_flush_plug_list() queue_unplugged() does not get invoked. Hence > >> blk_run_queue_async is not called. > >> Instead __elv_add_request() is invoked with ELEVATOR_INSERT_SORT_MERGE > >> flag and the flag gets reassigned to ELEVATOR_INSERT_BACK. > >> > >> In ELEVATOR_INSERT_BACK, __blk_run_queue() gets invoked and calls request_fn(). > > > This doesn't make sense. why the flag is changed from > > ELEVATOR_INSERT_SORT_MERGE to ELEVATOR_INSERT_BACK? > > In __elv_add_request() "where" gets reassigned as follows: > > } else if (!(rq->cmd_flags & REQ_ELVPRIV) && > (where == ELEVATOR_INSERT_SORT || > where == ELEVATOR_INSERT_SORT_MERGE)) > where = ELEVATOR_INSERT_BACK; > ok, thanks. So this means the elevator is switching in the test. How about below patch: diff --git a/block/elevator.c b/block/elevator.c index a3b64bc..e14824a 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -683,8 +683,13 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where) * - Usually, back inserted requests won't be merged * with anything. There's no point in delaying queue * processing. + * If elevator is switching, doesn't need run the queue. + * elevator switching will run it anyway. And this could + * cause warning since the code might run in atomic + * environment(blk_flush_plug_list() callbed in schedule()) */ - __blk_run_queue(q); + if (!test_bit(QUEUE_FLAG_ELVSWITCH, &q->queue_flags)) + __blk_run_queue(q); break; case ELEVATOR_INSERT_SORT_MERGE: