Re: Adding kmap_atomic_prot_pfn

From: Thomas Hellström
Date: Fri Oct 24 2008 - 07:10:35 EST


Ingo Molnar wrote:
* Thomas Hellström <thomas@xxxxxxxxxxxxxxxxxxxx> wrote:

Ingo Molnar wrote:
* Thomas Hellström <thomas@xxxxxxxxxxxxxxxxxxxx> 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





--
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/