Re: [PATCH 2/2] ttm: Fix ttm in-kernel copying of pages with non-standard caching attributes.

[Pekka Paalanen <pq@iki.fi>]

On Fri, 31 Jul 2009 10:59:57 +0200
Thomas Hellström <thomas@shipmail.org> wrote:

> Pekka Paalanen wrote:
> >> @@ -145,17 +146,35 @@ static int ttm_copy_io_ttm_page(struct ttm_tt *ttm, void *src,
> >>  		return -ENOMEM;
> >>  
> >>  	src = (void *)((unsigned long)src + (page << PAGE_SHIFT));
> >> -	dst = kmap(d);
> >> +
> >> +#ifdef CONFIG_X86
> >> +	dst = kmap_atomic_prot(d, KM_USER0, prot);
> >> +#else
> >> +	if (prot != PAGE_KERNEL)
> >> +		dst = vmap(&d, 1, 0, prot);
> >> +	else
> >> +		dst = kmap(d);
> >> +#endif
> >>     
> >
> > What are the implications of choosing the non-CONFIG_X86 path
> > even on x86?
> >   
> 
> The only implication is a slowdown if dealing with highmem pages or 
> pages with
> a non standard caching policy. Also you need the patch I just posted to 
> dri-devel / lkml to make it compile.
> I should've done more thorough testing of the non-x86 path.
> 
> > Is kmap_atomic_prot() simply an optimization allowed by the x86
> > arch, and the alternate way also works, although it uses the
> > precious vmalloc address space?
> >   
> 
> Exactly, although it's only using one page out of vmalloc space and for 
> the time it
> takes to copy a page to / from io.
> 
> > Since kmap_atomic_prot() is not exported on earlier kernels,
> > I'm tempted to just do the non-CONFIG_X86 path.
> >   
> For compat I think that should be fine. If your driver is using 
> accelerated copy to / from
> VRAM, you shouldn't even hit this path.

Okay, thank you very much.

-- 
Pekka Paalanen
http://www.iki.fi/pq/