On Tue, Jun 5, 2018 at 5:08 PM, Jerome Glisse <jglisse@xxxxxxxxxx> wrote: > On Tue, Jun 05, 2018 at 04:06:12PM -0700, Dan Williams wrote: [..] >> I want the EXPORT_SYMBOL_GPL on devm_memremap_pages() primarily for >> development purposes. Any new users of devm_memremap_pages() should be >> aware that they are subscribing to the whims of the core-VM, i.e. the >> ongoing evolution of 'struct page', and encourage those drivers to be >> upstream to improve the implementation, and consolidate use cases. I'm >> not qualified to comment on your "nor will it change anyone's legal >> position.", but I'm saying it's in the Linux kernel's best interest >> that new users of this interface assume they need to be GPL. > > Note that HMM isolate the device driver from struct page as long as > the driver only use HMM helpers to get to the information it needs. > I intend to be pedantic about that with any driver using HMM. I want > HMM to be an impedance layer that provide stable and simple API to > device driver while preserving freedom of change to mm. > I would not classify redefining put_page() and recompiling the entirety of the kernel to turn on HMM as "isolating the driver from 'struct page'". HMM is instead isolating these out of drivers from ever needing to go upstream. Unless the nouveau patches are using the entirety of what is already upstream for HMM, we should look to pare HMM back. There is plenty of precedent of building a large capability out-of-tree and piecemeal merging it later, so I do not buy the "chicken-egg" argument. The change in the export is to make sure we don't repeat this backward "merge first, ask questions later" mistake in the future as devm_memremap_pages() is continuing to find new users like peer-to-peer DMA support and Linux is better off if that development is upstream. From a purely technical standpoint devm_memremap_pages() is EXPORT_SYMBOL_GPL because it hacks around several implementation details in the core kernel to achieve its goal, and it leaks new assumptions all over the kernel. It is strictly not a self contained interface.
