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

["Thomas Hellström" <thomas@tungstengraphics.com>]

Arjan van de Ven wrote:
> Thomas Hellström wrote:
>> Arjan van de Ven wrote:
>>> Thomas Hellström wrote:
>>>> Given this problem, the previously mentioned use-case, and the fact 
>>>> that we mostly really use user-space mappings,
>>>> Is there a possibility we could add the following functions to 
>>>> Dave's patch (provided they would work as intended, of course, 
>>>> namely invalidate / bring back the kernel mapping).
>>>
>>> sadly there are multiple mappings, both in theory and practice.
>>> Especially the _np / _p functions specifically work on only the 
>>> mapping you specify.
>>>
>>> For this to work we would need to somehow make a "mark all mappings 
>>> NP, but please only do the kernel ones" kind of thing.
>>> The semantics of that are... lets say messy at best.
>> Hmm, I'm not sure I follow you here. Are you saying that it's illegal 
>> to have an NP mapping of a page (which, If I understand it correctly, 
>> means no mapping at all) at the same time as you have a, say 
>> user-space WC mapping pointing to the same physical page?
>
> no.
> What I'm saying is that even if you make one mapping NP, it's hard to 
> know you got all of them,
> even just the kernel one.
>
>>
>> I was under the impression that calling CPA on the kernel mapping of 
>> that page would do the rest?
>
> this is not a correct assumption in general, especially not for things 
> like
> "present".
> If a page is mapped 3 times in the kernel, and you set one to "np", 
> the others
> WILL stay mapped.
>
> (and CPA itself no longer exists period ;)
But what mappings are there,  immediately after alloc_page(), that 
set_memory_np won't catch? Certainly kmap_atomic() used to leave some 
stale mappings floating around, but that has been fixed as far as I can 
tell, and I guess we must be able to assume that a driver keeps track of 
its own kernel mappings and kill them before calling set_memory_np.

Drivers relying on set_memory_uc touching all mappings the driver hasn't 
set up itself must then have the same problem and needs to be fixed; 
referring in particular to agpgart for which driver the old CPA 
functionality was once created, IIRC.

We should probably get this right as soon as possible, as the agpgart 
driver is (and has for the last couple of years been) relying on wc / uc 
aliasing to be legal. If we can fix this with set_memory_np / 
set_memory_p that would be great, and would have some additional 
benefits for the drm memory manager as well.

/Thomas