svn commit: r289457 - head/sys/x86/x86

[Andriy Gapon]

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