Re: [PATCH] x86: create array based interface to change page attribute

Jesse Barnes
2008-04-07T18:25:28Z

On Wednesday, April 02, 2008 10:57 am Thomas Hellström wrote:
> Arjan van de Ven wrote:
> > Thomas Hellström wrote:
> > > to fix the long standing uc/wc aliasing issue, provided we
> >
> > I'm not opposed to a real fix. I am opposed to a bad hack.
>
> Great. So a real clean fix involves setting all "default" kernel
> mappings either to WC (which will require PAT) or
> Unmapped, for a pool of pages used in the graphics tables.
>
> To reduce the number of attribute changes for mappings that are
> frequently switched, and also to reduce the number of clflushes, and to
> avoid waiting for upcoming wc versions of set_memory_xx, I have a strong
> preference for unmapping the pages.

Hopefully the WC stuff will be upstream right after 2.6.25 comes out. Any
reason why we shouldn't keep the pages mapped in the kernel as WC assuming
the interface is there?

And we really should be keeping pools of pages around with the right type--we
don't want to change attributes any more than absolutely necessary (the ia64
uncached allocator does this right already, and in the DRM we actually keep
the mappings around right now afaict). We can allocate & free large chunks
at a time to deal with memory pressure one way or another...

> 3) Have code in x86/pageattr.c decide which "default" mappings are
> present on the given pages and set them all as non-present.
> In fact, there is already such a function in pageattr.c:
>
> kernel_map_pages(struct page *pages, int numpages, bool enable);
>
> But it's for debugging purposes only, could we use and export a variant
> of this?
>
> I guess I need a hint as to what's considered allowable here, to avoid
> spending a lot of time on something that will in the end get rejected
> anyway.

I think we do want an interface like this, even if only for graphics memory
(though I suspect some other device might like it as well). We'll also want
to do it at runtime periodically to allocate new hunks of memory for graphics
use, so a boot-time only thing won't work.

Also, to make the API readable, we'd probably want to split the function into
kernel_map_pages(..., enum memory_type type) and kernel_unmap_pages(...)
(though like I said I think we really should be mapping them WC not umapping
them altogether, since we do want to hit the ring buffer from the kernel with
the WC type for example).

Question is, will kernel_map_pages catch all the various kernel mappings
(regular identity map, large page text map,e tc.), perform the proper
flushing, and generally make sure we don't machine check on all platforms?

Jesse