Re: [PATCH 04/15] swiotlb: remove unnecessary swiotlb_bus_to_virt

[FUJITA Tomonori]

On Mon, 13 Jul 2009 21:17:21 -0500
Becky Bruce <beckyb@xxxxxxxxxxxxxxxxxxx> wrote:

> 
> On Jul 9, 2009, at 8:04 PM, FUJITA Tomonori wrote:
> 
> > swiotlb_bus_to_virt is unncessary; we can use swiotlb_bus_to_phys and
> > phys_to_virt instead.
> 
> phys_to_virt (also, virt_to_phys) is invalid for highmem addresses on  
> ppc.  In most of the uses in this file, it doesn't matter, as the  
> iotlb buffers themselves are alloc'd out of lowmem,

Right,

> but the  
> dma_mark_clean() calls could happen on a highmem addr.  Currently, on  
> powerpc, dma_mark_clean() doesn't do anything, so it isn't a  
> functional problem.

Oops, I overlooked this. However, as you said, this is not a problem
with the current code.


>  I'm fine with the bulk of this patch, because in  
> reality, if I need to do something with a virtual address of a highmem  
> page, I have to get a kmap for the page.  So the existing  
> swiotlb_bus_to_virt isn't really helping.
> 
> What is dma_mark_clean used for?  Could it be made to take a paddr,  
> and let the implementation deal with making sure there's a valid vaddr  
> for the actual "clean" operation?

I think that it's IA64's optimization (it's a NULL function on X86
like POWERPC). It's defined in arch/ia64/mm/init.c. Looks like POWERPC
could use this optimization, I guess.

dma_mark_clean() just modifies the page flag (what it needs is struct
page) so I think that we can make it take a paddr.


> I'd also like to see a comment with swiotlb_virt_to_bus() that notes  
> that it should only be called on addresses in lowmem.

Ok, I'll do.


Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/