From owner-svn-src-all@freebsd.org Fri Oct 23 11:36:14 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BE459A1C8D6; Fri, 23 Oct 2015 11:36:14 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 6550A3A7; Fri, 23 Oct 2015 11:36:12 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id OAA09712; Fri, 23 Oct 2015 14:36:11 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1Zpadn-000A2q-2K; Fri, 23 Oct 2015 14:36:11 +0300 Subject: Re: svn commit: r289457 - head/sys/x86/x86 To: "Jason A. Harmening" , src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org References: <201510171458.t9HEwtYS067933@repo.freebsd.org> From: Andriy Gapon X-Enigmail-Draft-Status: N1110 Message-ID: <562A1B73.5040205@FreeBSD.org> Date: Fri, 23 Oct 2015 14:35:15 +0300 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <201510171458.t9HEwtYS067933@repo.freebsd.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Oct 2015 11:36:14 -0000 On 17/10/2015 17:58, Jason A. Harmening wrote: > Author: jah > Date: Sat Oct 17 14:58:55 2015 > New Revision: 289457 > URL: https://svnweb.freebsd.org/changeset/base/289457 > > Log: > Don't page-align the physical address when calling PHYS_TO_VM_PAGE(). > > M busdma_bounce.c > > Modified: > head/sys/x86/x86/busdma_bounce.c > > Modified: head/sys/x86/x86/busdma_bounce.c > ============================================================================== > --- head/sys/x86/x86/busdma_bounce.c Sat Oct 17 14:48:39 2015 (r289456) > +++ head/sys/x86/x86/busdma_bounce.c Sat Oct 17 14:58:55 2015 (r289457) > @@ -1006,7 +1006,8 @@ add_bounce_page(bus_dma_tag_t dmat, bus_ > bpage->busaddr |= addr & PAGE_MASK; > } > bpage->datavaddr = vaddr; > - bpage->datapage = PHYS_TO_VM_PAGE(addr & ~PAGE_MASK); > + /* PHYS_TO_VM_PAGE() will truncate unaligned addresses. */ > + bpage->datapage = PHYS_TO_VM_PAGE(addr); > bpage->dataoffs = addr & PAGE_MASK; > bpage->datacount = size; > STAILQ_INSERT_TAIL(&(map->bpages), bpage, links); > I agree with the essence of the change - applying the page mask was completely redundant. But I do not agree with the comment. I know what you want say there, but what the comment is actually saying is more confusing than having no comment at all (IMO, of course). Consider this description of PHYS_TO_VM_PAGE: PHYS_TO_VM_PAGE() returns a vm_page_t object that represents a memory page to which the given physical address belongs. Why is the truncation needed? What does it do? What's the problem with unaligned addresses? It looks like the comment talks about a (possible) minor implementation detail of PHYS_TO_VM_PAGE(). It seems that the comment has proliferated into more code since this commit. P.S. It would be better, of course, if PHYS_TO_VM_PAGE had a manual page or at least a comment near its declaration. -- Andriy Gapon