On Sun, 2017-10-01 at 10:04 +0200, Christoph Hellwig wrote:

On Wed, Sep 27, 2017 at 11:23:52AM +0100, Robin Murphy wrote:

I found that debug_dma_alloc_coherent() and debug_dma_free_coherent() assume that dma_alloc_coherent() always returns a linear address.
However it's possible that dma_alloc_coherent() returns a non-linear address. In this case, page_to_pfn(virt_to_page(virt)) will return an incorrect pfn. If the pfn is valid and mapped as a COW page,
we will hit the warning when doing wp_page_copy().

Hmm, can the debug code assume anything? Right now you're just patching
it from supporting linear and vmalloc. But what about other
potential mapping types?

thanks for the review.

ARCHs like metag and xtensa define their mappings (non-vmalloc and
non-linear) for dma allocation.
These mapping types are architecture-dependent and should not be used
outside arch folders. So it is hard to check the mappings and convert
a virtual address to a correct pfn in lib/dam-debug.c

How about recording only vmalloc (by is_vmalloc_addr()) and linear
address (by virt_addr_valid()) in lib/dma-debug? Since current
implementation is not correct for those ARCHs.

if (!is_vmalloc_addr(addr) && !virt_addr_valid(addr))
return;

or

every ARCH should define its own dmava-to-pfn API to convert
a dma-allocted virtual address to a correct pfn and lib/dma-debug.c
can use that API directly. (long-term)

+ entry->pfn = is_vmalloc_addr(virt) ? vmalloc_to_pfn(virt) :
+ page_to_pfn(virt_to_page(virt));

Please use normal if/else conditionsals:

Is this for better readability? I'll send another patch for this.


thanks

if (is_vmalloc_addr(virt))
entry->pfn = vmalloc_to_pfn(virt);
else
entry->pfn = page_to_pfn(virt_to_page(virt));

+ .pfn = is_vmalloc_addr(virt) ? vmalloc_to_pfn(virt) :
+ page_to_pfn(virt_to_page(virt)),

Same here.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)