From patchwork Sun Jun 25 09:26:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ondrej Zary X-Patchwork-Id: 9808007 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BD033603F3 for ; Sun, 25 Jun 2017 09:26:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9A1E8285E3 for ; Sun, 25 Jun 2017 09:26:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8ACB8283C3; Sun, 25 Jun 2017 09:26:54 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2FCE7283C3 for ; Sun, 25 Jun 2017 09:26:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751325AbdFYJ0k (ORCPT ); Sun, 25 Jun 2017 05:26:40 -0400 Received: from smtp-1b.atlantis.sk ([80.94.52.26]:57156 "EHLO smtp-1b.atlantis.sk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751321AbdFYJ0j (ORCPT ); Sun, 25 Jun 2017 05:26:39 -0400 Received: from [192.168.0.2] (188-167-69-119.dynamic.chello.sk [188.167.69.119]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp-1b.atlantis.sk (Postfix) with ESMTPSA id 3F346804E642; Sun, 25 Jun 2017 11:26:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=rainbow-software.org; s=atlsmtp; t=1498382791; bh=Hca7cQ9MUWzN3+bfQCIXn1cVUTZBfyoBgh6AAaPv49s=; h=From:To:Subject:Date:Cc:References:In-Reply-To; b=Yc/uIZqgaKPvEbGYu48Lkw8vlpj8yilGg10HvRCLHlnyBQNz8qd76VE644RSNQIQd 0BrfKSsQCtfHGPtOE2fm9zdD5+40MpOTSpMeuUdZLiChZ0lgq1p7KA+feYhJCcFwRt VIxedZzGiTCoM9pq+31P+kDWuRWznz2KPrvQam8U= From: Ondrej Zary To: Finn Thain Subject: Re: [PATCH v2 0/5] g_NCR5380: PDMA fixes and cleanup Date: Sun, 25 Jun 2017 11:26:23 +0200 User-Agent: KMail/1.9.10 (enterprise35 0.20100827.1168748) Cc: "James E.J. Bottomley" , "Martin K. Petersen" , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, Michael Schmitz References: In-Reply-To: X-KMail-QuotePrefix: > MIME-Version: 1.0 Content-Disposition: inline X-Length: 2721 X-UID: 32 Message-Id: <201706251126.23739.linux@rainbow-software.org> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Saturday 24 June 2017 08:37:36 Finn Thain wrote: > Ondrej, would you please test this new series? > > Changed since v1: > - PDMA transfer residual is calculated earlier. > - End of DMA flag check is now polled (if there is any residual). > > > Finn Thain (2): > g_NCR5380: Limit sg_tablesize to avoid PDMA read overruns on DTC436 > g_NCR5380: Cleanup comments and whitespace > > Ondrej Zary (3): > g_NCR5380: Fix PDMA transfer size > g_NCR5380: End PDMA transfer correctly on target disconnection > g_NCR5380: Re-work PDMA loops > > drivers/scsi/g_NCR5380.c | 231 > ++++++++++++++++++++++------------------------- 1 file changed, 107 > insertions(+), 124 deletions(-) It mostly works, but there are some problems: It's not reliable - we continue the data transfer after poll_politely2 returns zero but we don't know if it returned because of host buffer being ready of because of an IRQ. So if a device disconnects during write, we continue to fill the buffer and only then find out that wait for 53c80 registers timed out. Then PDMA gets disabled: [ 3458.774251] sd 2:0:1:0: [sdb] tag#0 53c80 registers not accessible, device will be reset [ 3458.774251] sd 2:0:1:0: [sdb] tag#0 switching to slow handshake We can just reset and continue with a new PDMA transfer. Found no problems with reads. But when this happens during a write, we might have lost some data buffers that we need to transfer again. The chip's PDMA block counter does not seem to be very helpful here - testing shows that either one buffer is missing in the file or is duplicated. That's why my code had separate host buffer ready and IRQ checks. Host buffer first - if it's ready, transfer the data. If not, check for IRQ - if it was an error, rollback 2 buffers (the same if the host buffer is not ready in time). There's also a performance regression on DTC436 - the sg_tablesize limit affects performance badly. Before: # hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 4 MB in 3.21 seconds = 1.25 MB/sec Now: # hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 4 MB in 4.89 seconds = 837.69 kB/sec We should limit the transfer size instead: No data corruption and no performance regression: # hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 4 MB in 3.25 seconds = 1.23 MB/sec As the data corruption affects only writes, we could keep transfersize unlimited for reads: + /* Limit write transfers to 512B to prevent random corruption on DTC */ + if (hostdata->board == BOARD_DTC3181E && + cmd->sc_data_direction == DMA_TO_DEVICE && transfersize > 512) + transfersize = 512; So we can get some performance gain at least for reads: # hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 6 MB in 4.17 seconds = 1.44 MB/sec --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -45,7 +45,8 @@ int c400_blk_cnt; \ int c400_host_buf; \ int io_width; \ - int pdma_residual + int pdma_residual; \ + int board; #define NCR5380_dma_xfer_len generic_NCR5380_dma_xfer_len #define NCR5380_dma_recv_setup generic_NCR5380_pread @@ -247,7 +248,6 @@ static int generic_NCR5380_init_one(struct scsi_host_template *tpnt, case BOARD_DTC3181E: ports = dtc_3181e_ports; magic = ncr_53c400a_magic; - tpnt->sg_tablesize = 1; break; } @@ -317,6 +317,7 @@ static int generic_NCR5380_init_one(struct scsi_host_template *tpnt, } hostdata = shost_priv(instance); + hostdata->board = board; hostdata->io = iomem; hostdata->region_size = region_size; @@ -625,6 +626,9 @@ static int generic_NCR5380_dma_xfer_len(struct NCR5380_hostdata *hostdata, /* 53C400 datasheet: non-modulo-128-byte transfers should use PIO */ if (transfersize % 128) transfersize = 0; + /* Limit transfers to 512B to prevent random write corruption on DTC */ + if (hostdata->board == BOARD_DTC3181E && transfersize > 512) + transfersize = 512; return min(transfersize, DMA_MAX_SIZE); }