Re: [PATCH v3] dma-debug: fix incorrect pfn calculation

From: Christoph Hellwig
Date: Tue Oct 03 2017 - 03:07:59 EST


On Mon, Oct 02, 2017 at 06:30:41PM +0800, Miles Chen wrote:
> ARCHs like metag and xtensa define their mappings (non-vmalloc and
> non-linear) for dma allocation.

metag basically is a reimplementation of the vmalloc map mechanism
that should be easy to consolidate into the common one :( xtensa
has a weird remapping into a different segment, something that I
vaguely remember mips used to support as well.

> 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

This looks like a good start, although I'm not sure I'd trust
virt_addr_valid on every little arch. In the worse case we'll
have to exclude offenders from supporting dma debug, so let's go
with that version.

> > > > + 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.

Yes.