From patchwork Wed Jul 28 10:46:26 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dominic Curran X-Patchwork-Id: 60132 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 1D12FB6EDF for ; Wed, 28 Jul 2010 20:50:21 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754623Ab0G1KuT (ORCPT ); Wed, 28 Jul 2010 06:50:19 -0400 Received: from smtp.citrix.com ([66.165.176.89]:9754 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754397Ab0G1KuT (ORCPT ); Wed, 28 Jul 2010 06:50:19 -0400 X-Greylist: delayed 830 seconds by postgrey-1.27 at vger.kernel.org; Wed, 28 Jul 2010 06:50:19 EDT X-IronPort-AV: E=Sophos;i="4.55,273,1278302400"; d="scan'208";a="9540253" Received: from ftlpmailmx02.citrite.net ([10.9.154.224]) by FTLPIPO01.CITRIX.COM with ESMTP/TLS/RC4-MD5; 28 Jul 2010 06:36:28 -0400 Received: from [10.80.3.124] (10.9.154.239) by FTLPMAILMX02.citrite.net (10.9.154.224) with Microsoft SMTP Server id 8.2.254.0; Wed, 28 Jul 2010 06:36:28 -0400 Message-ID: <4C500A82.2040805@citrix.com> Date: Wed, 28 Jul 2010 11:46:26 +0100 From: Dominic Curran User-Agent: Thunderbird 2.0.0.24 (X11/20100411) MIME-Version: 1.0 To: "linux-ide@vger.kernel.org" Subject: Possible bug in ide_cd_queue_pc() or ide_wait_stat() ? Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org I have some TEAC DV-28E-V CDROM drives that after polling for status starts to timeout requests (the time it takes to do this varies between 5mins and 24hrs). I believe only an APATPI reset gets them out of this timeout behaviour. I assume this is down to bad firmware/hardware (the latest firmware has been applied). I happens on multiply drives of this version. BUT... The problem is that even when the requests timeout the ioctl CDROM_DRIVE_STATUS will receive a status of CDS_DISC_OK. This is obviously not the correct status (particularly when there is no CD disc in the drive). I have added some instrumentation to the code and this is the general stack flow I see: In the interrupt handler: do_ide_request() ide_do_request() start_request() returns ide_stopped ide_wait_stat() returns -EBUSY ide_error() returns ide_stopped + stat=0xD0 {Busy} ide_dump_status() returns 0 (so err=0) rq->errors = 1; ide_end_drive_cmd(err) b/c err is 0, then sets rq->errors=0 In the ioctl: ide_cdrom_drive_status() Returns CDS_DISC_OK cdrom_check_status() Returns 0. ide_cd_queue_pc() Ignores the return from blk_execute_rq(). Checks for the flag REQ_FAILED in rq->cmd_flags (which is not set). Returns 0. blk_execute_rq() Since rq->errors==1 and then returns 0 wait_for_completion() The problem seems to be that in ide_cd_queue_pc(): 1) the error return from blk_execute_rq() is ignored 2) the REQ_FAILED in rq->cmd_flags is not set in the interrupt handler (which is what ide_cd_queue_pc() seems to be concerned about) Anyone have any comments: 1) why in ide_dump_status() a stat of BUSY_STAT does not translate to an error return ? 2) Is setting rq->cmd_flags |= REQ_FAILED in ide_wait_stat() an acceptable way to solve issue. Patch something like this... I appreciate any pointers you can give Thanks dom --- 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 Index: linux/drivers/ide/ide-iops.c =================================================================== --- linux.orig/drivers/ide/ide-iops.c 2010-07-20 15:37:01.871300665 +0100 +++ linux/drivers/ide/ide-iops.c 2010-07-28 11:43:21.386752993 +0100 @@ -652,6 +657,9 @@ int ide_wait_stat(ide_startstop_t *start if (err) { char *s = (err == -EBUSY) ? "status timeout" : "status error"; *startstop = ide_error(drive, s, stat); + + if (err == -EBUSY) + rq->cmd_flags |= REQ_FAILED; } return err;