From patchwork Tue Feb 15 15:13:14 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 83262 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 5109CB711E for ; Wed, 16 Feb 2011 02:13:21 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751345Ab1BOPNU (ORCPT ); Tue, 15 Feb 2011 10:13:20 -0500 Received: from mail-fx0-f46.google.com ([209.85.161.46]:50003 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750857Ab1BOPNT (ORCPT ); Tue, 15 Feb 2011 10:13:19 -0500 Received: by fxm20 with SMTP id 20so288008fxm.19 for ; Tue, 15 Feb 2011 07:13:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:date:from:to:cc:subject:message-id :references:mime-version:content-type:content-disposition :in-reply-to:user-agent; bh=bXObPG03usGlM67LcGnQIiJdL1mxAbmiC868VFWzXBM=; b=jOZTVJxKZWPvJ/ijunXQpQjItlWOBedlzckDJcB0zfvYFPTa5naVLwTujxSNmAjnMe ajLHWJ8YBgImwJW0i1Q8IpGVqg/CGqAiuS5Q10y/zrgZ7pEnK+9CRY57g2sJ/QQeUOjx mK3qzk32z3a37heMQSufWH8J+I1FSTPLHnDQ4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=PBaoOmPSH0kriIosYSLV0zLc0gzAh/br8h1N3D1vvrpJU4m3SnDOz8V8TpXG43GAYm 7Is2x0nS7KVkSFrtAZsNF8FAtTu0VpB7BiYvtoBELGsHDTwL9K5mYEDG3oDzUrXWouaL dsAPnpeb1w6MyVsmlOoi8MnIYV70CYPrCkU64= Received: by 10.223.53.68 with SMTP id l4mr6411121fag.44.1297782798030; Tue, 15 Feb 2011 07:13:18 -0800 (PST) Received: from htj.dyndns.org ([130.75.117.88]) by mx.google.com with ESMTPS id 21sm1699391fav.17.2011.02.15.07.13.16 (version=SSLv3 cipher=OTHER); Tue, 15 Feb 2011 07:13:16 -0800 (PST) Date: Tue, 15 Feb 2011 16:13:14 +0100 From: Tejun Heo To: Jan Beulich Cc: linux-ide@vger.kernel.org Subject: Re: old ide driver incompatible with new flush implementation (was: conflicting commits (block flush vs. ide)) Message-ID: <20110215151314.GJ3160@htj.dyndns.org> References: <4D5A75650200007800031F9C@vpn.id2.novell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4D5A75650200007800031F9C@vpn.id2.novell.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org On Tue, Feb 15, 2011 at 11:45:25AM +0000, Jan Beulich wrote: > >>> On 15.02.11 at 10:35, Jan Beulich wrote: > > BUG: scheduling while atomic: swapper/0/0x10010000 > > Actually, the situation is worse. While the patch sent previously > allowed that system to boot, it died the moment it tried to > access its secondary disk (which supports flushes, while the > primary doesn't): > > BUG: scheduling while atomic: swapper/0/0x10010000 > Process swapper (pid: 0, ti=ec004000 task=c04c14e0 task.ti=c04b0000) > Stack: > Call Trace: > Code: cc cc cc cc b8 1c 00 00 00 cd 82 c3 cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc b8 1d 00 00 00 cd 82 cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc > Kernel panic - not syncing: scheduling while atomic > Pid: 0, comm: swapper Not tainted 2.6.37-2011-01-22-xen0 #4 > Call Trace: > [] try_stack_unwind+0x14b/0x170 > [] dump_trace+0x3f/0xf0 > [] show_trace_log_lvl+0x4b/0x60 > [] show_trace+0x18/0x20 > [] dump_stack+0x6d/0x72 > [] panic+0x57/0x15b > [] __schedule_bug+0x58/0x60 > [] schedule+0x466/0x5a0 > [] _cond_resched+0x2f/0x50 > [] do_ide_request+0x55/0x450 > [] __generic_unplug_device+0x28/0x30 > [] blk_flush_complete_seq+0xbc/0x1a0 -> queue_next_fseq() -> elv_insert() Yeah, right. blk_flush_complete_seq_end_io() is on completion path and shouldn't call directly into request_fn. Can you please test whether the following patch fixes the problem? Thanks. --- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/block/blk-core.c b/block/blk-core.c index 2f4002f..7f59923 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -352,7 +352,7 @@ void blk_start_queue(struct request_queue *q) WARN_ON(!irqs_disabled()); queue_flag_clear(QUEUE_FLAG_STOPPED, q); - __blk_run_queue(q); + __blk_run_queue(q, false); } EXPORT_SYMBOL(blk_start_queue); @@ -403,13 +403,14 @@ EXPORT_SYMBOL(blk_sync_queue); /** * __blk_run_queue - run a single device queue * @q: The queue to run + * @force_kblockd: Don't run @q->request_fn directly. Use kblockd. * * Description: * See @blk_run_queue. This variant must be called with the queue lock * held and interrupts disabled. * */ -void __blk_run_queue(struct request_queue *q) +void __blk_run_queue(struct request_queue *q, bool force_kblockd) { blk_remove_plug(q); @@ -423,7 +424,7 @@ void __blk_run_queue(struct request_queue *q) * Only recurse once to avoid overrunning the stack, let the unplug * handling reinvoke the handler shortly if we already got there. */ - if (!queue_flag_test_and_set(QUEUE_FLAG_REENTER, q)) { + if (!force_kblockd && !queue_flag_test_and_set(QUEUE_FLAG_REENTER, q)) { q->request_fn(q); queue_flag_clear(QUEUE_FLAG_REENTER, q); } else { @@ -446,7 +447,7 @@ void blk_run_queue(struct request_queue *q) unsigned long flags; spin_lock_irqsave(q->queue_lock, flags); - __blk_run_queue(q); + __blk_run_queue(q, false); spin_unlock_irqrestore(q->queue_lock, flags); } EXPORT_SYMBOL(blk_run_queue); @@ -1053,7 +1054,7 @@ void blk_insert_request(struct request_queue *q, struct request *rq, drive_stat_acct(rq, 1); __elv_add_request(q, rq, where, 0); - __blk_run_queue(q); + __blk_run_queue(q, false); spin_unlock_irqrestore(q->queue_lock, flags); } EXPORT_SYMBOL(blk_insert_request); diff --git a/block/blk-flush.c b/block/blk-flush.c index 54b123d..e0bfce8 100644 --- a/block/blk-flush.c +++ b/block/blk-flush.c @@ -66,10 +66,12 @@ static void blk_flush_complete_seq_end_io(struct request_queue *q, /* * Moving a request silently to empty queue_head may stall the - * queue. Kick the queue in those cases. + * queue. Kick the queue in those cases. This function is called + * from request completion path and calling directly into + * request_fn may confuse the driver. Always use kblockd. */ if (was_empty && next_rq) - __blk_run_queue(q); + __blk_run_queue(q, true); } static void pre_flush_end_io(struct request *rq, int error) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 7be4c79..ea83a4f 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -3355,7 +3355,7 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq, cfqd->busy_queues > 1) { cfq_del_timer(cfqd, cfqq); cfq_clear_cfqq_wait_request(cfqq); - __blk_run_queue(cfqd->queue); + __blk_run_queue(cfqd->queue, false); } else { cfq_blkiocg_update_idle_time_stats( &cfqq->cfqg->blkg); @@ -3370,7 +3370,7 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq, * this new queue is RT and the current one is BE */ cfq_preempt_queue(cfqd, cfqq); - __blk_run_queue(cfqd->queue); + __blk_run_queue(cfqd->queue, false); } } @@ -3731,7 +3731,7 @@ static void cfq_kick_queue(struct work_struct *work) struct request_queue *q = cfqd->queue; spin_lock_irq(q->queue_lock); - __blk_run_queue(cfqd->queue); + __blk_run_queue(cfqd->queue, false); spin_unlock_irq(q->queue_lock); } diff --git a/block/elevator.c b/block/elevator.c index 2569512..236e93c 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -602,7 +602,7 @@ void elv_quiesce_start(struct request_queue *q) */ elv_drain_elevator(q); while (q->rq.elvpriv) { - __blk_run_queue(q); + __blk_run_queue(q, false); spin_unlock_irq(q->queue_lock); msleep(10); spin_lock_irq(q->queue_lock); @@ -651,7 +651,7 @@ void elv_insert(struct request_queue *q, struct request *rq, int where) * with anything. There's no point in delaying queue * processing. */ - __blk_run_queue(q); + __blk_run_queue(q, false); break; case ELEVATOR_INSERT_SORT: diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 9045c52..fb2bb35 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -443,7 +443,7 @@ static void scsi_run_queue(struct request_queue *q) &sdev->request_queue->queue_flags); if (flagset) queue_flag_set(QUEUE_FLAG_REENTER, sdev->request_queue); - __blk_run_queue(sdev->request_queue); + __blk_run_queue(sdev->request_queue, false); if (flagset) queue_flag_clear(QUEUE_FLAG_REENTER, sdev->request_queue); spin_unlock(sdev->request_queue->queue_lock); diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index 998c01b..5c3ccfc 100644 --- a/drivers/scsi/scsi_transport_fc.c +++ b/drivers/scsi/scsi_transport_fc.c @@ -3829,7 +3829,7 @@ fc_bsg_goose_queue(struct fc_rport *rport) !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags); if (flagset) queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q); - __blk_run_queue(rport->rqst_q); + __blk_run_queue(rport->rqst_q, false); if (flagset) queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q); spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 4d18ff3..d5e444d 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -699,7 +699,7 @@ extern void blk_start_queue(struct request_queue *q); extern void blk_stop_queue(struct request_queue *q); extern void blk_sync_queue(struct request_queue *q); extern void __blk_stop_queue(struct request_queue *q); -extern void __blk_run_queue(struct request_queue *); +extern void __blk_run_queue(struct request_queue *q, bool force_kblockd); extern void blk_run_queue(struct request_queue *); extern int blk_rq_map_user(struct request_queue *, struct request *, struct rq_map_data *, void __user *, unsigned long,