Re: Adding kmap_atomic_prot_pfn

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

Ingo Molnar wrote:
> * Thomas Hellström <thomas@tungstengraphics.com> wrote:
>
>   
>> Ingo Molnar wrote:
>>     
>>> * Thomas Hellström <thomas@tungstengraphics.com> wrote:
>>>
>>>   
>>>       
>>>> Keith,
>>>>
>>>> What you actually are doing here is claiming copyright on code that  
>>>> other people have written, and tighten the export restrictions.   
>>>> kmap_atomic_prot_pfn() appeared long ago in drm git with identical  
>>>> code and purpose, but with different authors, and iounmap_atomic is  
>>>> identical to kunmap_atomic.
>>>>     
>>>>         
>>>   
>>>       
>>>>> +EXPORT_SYMBOL_GPL(iomap_atomic_prot_pfn);
>>>>>       
>>>>>           
>>> you want to use this facility in a binary-only driver?
>>>
>>> 	Ingo
>>>   
>>>       
>> At this point I have no such use for it, no.
>> The original user was a MIT style licenced driver.
>>     
>
> okay, then just to put this question to rest: i wrote the original 
> 32-bit highmem code ~10 years ago. I wrote the first version of fixmap 
> support - in fact i coined the term. I wrote the first version of the 
> atomic-kmap facility as well.
>
> All of that code is licensed under the GPLv2. So if anyone wants to make 
> any copyright claims about highmem/kmap/fixmap derivative works, 
> consider it in that light.
>   
I fully acknowledge that. The fact that others wrote almost all of the 
code is the reason why there is no  additional copyright claims in the 
file of  the original kmap_atomic_prot_pfn() implementation.

What I'm considering very bad form is someone taking other people's 
contributions, be it code or ideas,  no matter how small they are, claim 
copyright on them and restrict the original usage. That's really the 
point here. Frankly, from my own usage point of view I don't really care 
about the specific case (whether they are exported GPL or not), but with 
the current form of this patch, I'm basically no longer allowed to use 
the code currently in DRM git without Keith's permission.

> Regarding this new API variant that Keith wrote: it would be silly and 
> dangerous to export it anywhere but to in-kernel drivers. The API 
> disables preemption on 32-bit and rummages deep in the guts of the 
> kernel as well, uses up a precious resource (fixmap slots), etc. It's 
> internal and we eventually might want to deprecate forms of it and 
> concentrate on the good 64-bit performance side.
>
>   
These are all good arguments to reserve this api for in-kernel drivers.

There are other reasons why it should be available to out of tree drivers:

* The api enables a fast facility that will be extremely important by 
graphics drivers in the future, probably not only for in-kernel drivers.
Particularly as future graphics drivers will want to get fast 
write-combined kernel mappings also on highmem pages, not only io.
This latter actually suggests keeping the form kmap_atomic_prot_pfn 
instead of iomap_atomic_prot_pfn, and make it permanent, unless the same 
goals can be achieved by the VMAP work Nick Piggin is suggesting.

* The use of k[un]map_atomic() would, following your argumentation, be 
equally dangerous to export non-GPL?

Now, considering these pros and cons one might still come to the 
conclusion that for stability reasons it is best to keep this API for in 
kernel drivers. I don't really know enough about the affected kernel 
internals to be able to judge. I just think it's important that all 
facts are considered.

/Thomas