From owner-freebsd-current@FreeBSD.ORG Thu Apr 16 17:39:35 2009 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6B173106567B; Thu, 16 Apr 2009 17:39:35 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 2DC1B8FC16; Thu, 16 Apr 2009 17:39:35 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id B8C9646B3B; Thu, 16 Apr 2009 13:39:34 -0400 (EDT) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id A76178A01B; Thu, 16 Apr 2009 13:39:33 -0400 (EDT) From: John Baldwin To: current@FreeBSD.org Date: Thu, 16 Apr 2009 13:36:18 -0400 User-Agent: KMail/1.9.7 MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200904161336.18557.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 16 Apr 2009 13:39:33 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=0.1 required=4.2 tests=RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: hps@FreeBSD.org, rnoland@freebsd.org Subject: [PATCH] Possible fix to recent data corruption on HEAD since USB2 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Apr 2009 17:39:38 -0000 Due to some good sleuthing by avg@, there is a patch that might fix the recent reports of data corruption on current. It would explain some of the recent reports where a file that was read would have missing gaps of bytes. The problem is with the BUS_DMA_KEEP_PG_OFFSET changes to bus_dma. When a bounce page was used by USB2, the changes to bus_dma would actually change the starting virtual and physical addresses of the bounce page. When the bounce page was no longer needed it was left in this bogus state. Later if another device used the same bounce page for DMA it would use the wrong offset and address. The issue there is if the second device was doing a full page of I/O. In that case the DMA from the device would actually spill over into the next page which could in theory be used by another DMA request. It could also break alignment assumptions (since the previous PG_OFFSET may not be aligned and the bus_dma code assumes bounce pages for the !PG_OFFSET case are page aligned). The quick fix is to always restore the bounce page to the normal state when a PG_OFFSET DMA request is finished. I'd actually prefer not ever touching the page's starting addresses, but those changes would be more invasive I believe. http://www.FreeBSD.org/~jhb/patches/dma_sg.patch Index: amd64/amd64/busdma_machdep.c =================================================================== --- amd64/amd64/busdma_machdep.c (revision 191101) +++ amd64/amd64/busdma_machdep.c (working copy) @@ -1138,8 +1138,6 @@ if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { /* page offset needs to be preserved */ - bpage->vaddr &= ~PAGE_MASK; - bpage->busaddr &= ~PAGE_MASK; bpage->vaddr |= vaddr & PAGE_MASK; bpage->busaddr |= vaddr & PAGE_MASK; } @@ -1158,6 +1156,10 @@ bz = dmat->bounce_zone; bpage->datavaddr = 0; bpage->datacount = 0; + if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { + bpage->vaddr &= ~PAGE_MASK; + bpage->busaddr &= ~PAGE_MASK; + } mtx_lock(&bounce_lock); STAILQ_INSERT_HEAD(&bz->bounce_page_list, bpage, links); Index: arm/arm/busdma_machdep.c =================================================================== --- arm/arm/busdma_machdep.c (revision 191101) +++ arm/arm/busdma_machdep.c (working copy) @@ -1428,8 +1428,6 @@ if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { /* page offset needs to be preserved */ - bpage->vaddr &= ~PAGE_MASK; - bpage->busaddr &= ~PAGE_MASK; bpage->vaddr |= vaddr & PAGE_MASK; bpage->busaddr |= vaddr & PAGE_MASK; } @@ -1448,6 +1446,10 @@ bz = dmat->bounce_zone; bpage->datavaddr = 0; bpage->datacount = 0; + if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { + bpage->vaddr &= ~PAGE_MASK; + bpage->busaddr &= ~PAGE_MASK; + } mtx_lock(&bounce_lock); STAILQ_INSERT_HEAD(&bz->bounce_page_list, bpage, links); Index: i386/i386/busdma_machdep.c =================================================================== --- i386/i386/busdma_machdep.c (revision 191101) +++ i386/i386/busdma_machdep.c (working copy) @@ -1156,8 +1156,6 @@ if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { /* page offset needs to be preserved */ - bpage->vaddr &= ~PAGE_MASK; - bpage->busaddr &= ~PAGE_MASK; bpage->vaddr |= vaddr & PAGE_MASK; bpage->busaddr |= vaddr & PAGE_MASK; } @@ -1176,6 +1174,10 @@ bz = dmat->bounce_zone; bpage->datavaddr = 0; bpage->datacount = 0; + if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { + bpage->vaddr &= ~PAGE_MASK; + bpage->busaddr &= ~PAGE_MASK; + } mtx_lock(&bounce_lock); STAILQ_INSERT_HEAD(&bz->bounce_page_list, bpage, links); Index: ia64/ia64/busdma_machdep.c =================================================================== --- ia64/ia64/busdma_machdep.c (revision 191101) +++ ia64/ia64/busdma_machdep.c (working copy) @@ -941,8 +941,6 @@ if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { /* page offset needs to be preserved */ - bpage->vaddr &= ~PAGE_MASK; - bpage->busaddr &= ~PAGE_MASK; bpage->vaddr |= vaddr & PAGE_MASK; bpage->busaddr |= vaddr & PAGE_MASK; } @@ -959,6 +957,10 @@ bpage->datavaddr = 0; bpage->datacount = 0; + if (dmat->flags & BUS_DMA_KEEP_PG_OFFSET) { + bpage->vaddr &= ~PAGE_MASK; + bpage->busaddr &= ~PAGE_MASK; + } mtx_lock(&bounce_lock); STAILQ_INSERT_HEAD(&bounce_page_list, bpage, links); -- John Baldwin