Date: Fri, 23 Oct 2015 14:35:15 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: "Jason A. Harmening" <jah@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org Subject: Re: svn commit: r289457 - head/sys/x86/x86 Message-ID: <562A1B73.5040205@FreeBSD.org> In-Reply-To: <201510171458.t9HEwtYS067933@repo.freebsd.org> References: <201510171458.t9HEwtYS067933@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?562A1B73.5040205>