From patchwork Thu Apr 8 07:52:16 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 49704 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 85BC1B7D15 for ; Thu, 8 Apr 2010 17:49:00 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932243Ab0DHHs6 (ORCPT ); Thu, 8 Apr 2010 03:48:58 -0400 Received: from hera.kernel.org ([140.211.167.34]:50412 "EHLO hera.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932228Ab0DHHs6 (ORCPT ); Thu, 8 Apr 2010 03:48:58 -0400 Received: from htj.dyndns.org (localhost [127.0.0.1]) by hera.kernel.org (8.14.3/8.14.3) with ESMTP id o387mmSH011527 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Thu, 8 Apr 2010 07:48:49 GMT X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.95.2 at hera.kernel.org Received: from [192.168.0.3] (unknown [121.167.144.32]) by htj.dyndns.org (Postfix) with ESMTPSA id C41D9100B8A01; Thu, 8 Apr 2010 16:48:47 +0900 (KST) Message-ID: <4BBD8B30.6040407@kernel.org> Date: Thu, 08 Apr 2010 16:52:16 +0900 From: Tejun Heo User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1.9) Gecko/20100317 Thunderbird/3.0.4 MIME-Version: 1.0 To: "linux-ide@vger.kernel.org" , davem@davemloft.net CC: herbert@gondor.hengli.com.au Subject: [PATCH ide-2.6#master REPOST] ide: clean up timed out request handling X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on hera.kernel.org X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.3 (hera.kernel.org [127.0.0.1]); Thu, 08 Apr 2010 07:48:50 +0000 (UTC) Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org 8f6205cd572fece673da0255d74843680f67f879 introduced a bug where a timed out DMA request is never requeued and lost. 6072f7491f5ef391a575e18a1165e72a3eef1601 fixed this by making ide_dma_timeout_retry() requeue the request itself. While the fix is correct, it makes DMA and non-DMA paths asymmetric regarding how the in flight request is requeued. As long as hwif->rq is set, the IDE driver is assuming ownership of the request and the request should either be completed or requeued when clearing hwif->rq. In the timeout path, the ide driver holds onto the request as long as the recovery action (ie. reset) is in progress and clears it after the state machine is stopped (ide_stopped return), so the existing requeueing logic is correct. The bug occurred because ide_dma_timeout_retry() explicitly clears hwif->rq without requeueing it. ide_dma_timeout_retry() is called only by ide_timer_expiry() and returns ide_started only when ide_error() would return it - ie. after reset state machine has started in which case the state machine will eventually end up executing the ide_stopped path in ide_timer_expiry() after reset protocol is complete. So, there is no need to clear hwif->rq from ide_dma_timeout_retry(). ide_timer_expiry() will handle it the same way as PIO timeout path. Kill hwif->rq clearing and requeueing from ide_dma_timeout_retry() and let ide_timer_expiry() deal with it. The end result should remain the same. grepping shows ide_dma_timeout_retry() is the only site which clears hwif->rq without taking care of the request, so there shouldn't be similar fallouts. Signed-off-by: Tejun Heo Cc: Herbert Xu Cc: Bartlomiej Zolnierkiewicz --- David, here's the repost. Thanks. drivers/ide/ide-dma.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) -- 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/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c index fd40a81..963bc8b 100644 --- a/drivers/ide/ide-dma.c +++ b/drivers/ide/ide-dma.c @@ -448,7 +448,6 @@ ide_startstop_t ide_dma_timeout_retry(ide_drive_t *drive, int error) ide_hwif_t *hwif = drive->hwif; const struct ide_dma_ops *dma_ops = hwif->dma_ops; struct ide_cmd *cmd = &hwif->cmd; - struct request *rq; ide_startstop_t ret = ide_stopped; /* @@ -486,14 +485,10 @@ ide_startstop_t ide_dma_timeout_retry(ide_drive_t *drive, int error) ide_dma_off_quietly(drive); /* - * un-busy drive etc and make sure request is sane + * make sure request is sane */ - rq = hwif->rq; - if (rq) { - hwif->rq = NULL; - rq->errors = 0; - ide_requeue_and_plug(drive, rq); - } + if (hwif->rq) + hwif->rq->errors = 0; return ret; }