From patchwork Fri Jan 6 02:17:07 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 134583 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 3FCCAB6FA9 for ; Fri, 6 Jan 2012 13:17:42 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758541Ab2AFCRO (ORCPT ); Thu, 5 Jan 2012 21:17:14 -0500 Received: from mail-iy0-f174.google.com ([209.85.210.174]:60363 "EHLO mail-iy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758530Ab2AFCRM (ORCPT ); Thu, 5 Jan 2012 21:17:12 -0500 Received: by iaeh11 with SMTP id h11so1830929iae.19 for ; Thu, 05 Jan 2012 18:17:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; 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; bh=Bd9H3XG9kjVc8j5qAZaNwxGHQCbyfkunkOtqTFPg0I0=; b=vKj+ZvwW5O+pQzjDztj6t4TieRwy3xTbu0DCvT/B+7hq8EE1TZoNFiwX0PKl30XX9A cGN+a/QTYElzVw8WPbCtgZ1FPCJVRJXw/6c6qVl4WKdDVTvREh9PEgRr63nHwjPr7dzH ImZMgJKbcyYWO6bekfQF5BlDwGmuRf4Eh5bL8= Received: by 10.50.155.166 with SMTP id vx6mr5884885igb.16.1325816231928; Thu, 05 Jan 2012 18:17:11 -0800 (PST) Received: from google.com (wtj.mtv.corp.google.com [172.18.96.96]) by mx.google.com with ESMTPS id b20sm208040534ibj.7.2012.01.05.18.17.09 (version=SSLv3 cipher=OTHER); Thu, 05 Jan 2012 18:17:10 -0800 (PST) Date: Thu, 5 Jan 2012 18:17:07 -0800 From: Tejun Heo To: Jens Axboe , Shaohua Li , Hugh Dickins Cc: Andrew Morton , Stephen Rothwell , linux-next@vger.kernel.org, LKML , linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org, x86@kernel.org Subject: [PATCH block:for-3.3/core] cfq: merged request shouldn't jump to a different cfqq Message-ID: <20120106021707.GA6276@google.com> References: <20111228211918.GA3516@google.com> <20120103173500.GB31746@google.com> <20120103175922.GC31746@google.com> <20120103200906.GG31746@google.com> <4F03631C.8080501@kernel.dk> <20120103221301.GH31746@google.com> <20120103223505.GI31746@google.com> <20120105012445.GP31746@google.com> <20120105183842.GF18486@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20120105183842.GF18486@google.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 When two requests are merged, if the absorbed request is older than the absorbing one, cfq_merged_requests() tries to reposition it in the cfqq->fifo list by list_move()'ing the absorbing request to the absorbed one before removing it. This works if both requests are on the same cfqq but nothing guarantees that and the code ends up moving the merged request to a --- 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 different cfqq's fifo list without adjusting the rest. This leads to the following failures. * A request may be on the fifo list of a cfqq without holding reference to it and the cfqq can be freed before requst is finished. Among other things, this triggers list debug warning and slab debug use-after-free warning. * As a request can be on the wrong fifo queue, it may be issued and completed before its cfqq is scheduled. If the cfqq didn't have other requests on it, it would be empty by the time it's dispatched triggering BUG_ON() in cfq_dispatch_request(). Fix it by making cfq_merged_requests() scan the absorbing request's fifo list for the correct slot and move there instead. Signed-off-by: Tejun Heo Reported-by: Hugh Dickins Cc: stable@vger.kernel.org --- It survived my testing long enough and I'm relatively confident this should fix the crash but I might have gotten the scanning wrong, so please pay extra attention there. I suspect we just didn't have enough backward request-request merges before the recent plug merge updates to trigger this bug. Thanks. block/cfq-iosched.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 163263d..a235cf3 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1663,8 +1663,15 @@ cfq_merged_requests(struct request_queue *q, struct request *rq, */ if (!list_empty(&rq->queuelist) && !list_empty(&next->queuelist) && time_before(rq_fifo_time(next), rq_fifo_time(rq))) { - list_move(&rq->queuelist, &next->queuelist); + struct request *pos = rq; + rq_set_fifo_time(rq, rq_fifo_time(next)); + + list_for_each_entry_continue_reverse(pos, &cfqq->fifo, queuelist) + if (time_before_eq(rq_fifo_time(pos), rq_fifo_time(rq))) + break; + if (rq != pos) + list_move(&rq->queuelist, &pos->queuelist); } if (cfqq->next_rq == next)