diff mbox

[v3,7/8] mm, hmm: Mark hmm_devmem_{add, add_resource} EXPORT_SYMBOL_GPL

Message ID 152938831573.17797.15264540938029137916.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Williams June 19, 2018, 6:05 a.m. UTC
The routines hmm_devmem_add(), and hmm_devmem_add_resource() are
now wrappers around the functionality provided by devm_memremap_pages() to
inject a dev_pagemap instance and hook page-idle events. The
devm_memremap_pages() interface is base infrastructure for HMM which has
more and deeper ties into the kernel memory management implementation
than base ZONE_DEVICE.

Originally, the HMM page structure creation routines copied the
devm_memremap_pages() code and reused ZONE_DEVICE. A cleanup to unify
the implementations was discussed during the initial review:
http://lkml.iu.edu/hypermail/linux/kernel/1701.2/00812.html

Given that devm_memremap_pages() is marked EXPORT_SYMBOL_GPL by its
authors and the hmm_devmem_{add,add_resource} routines are simple
wrappers around that base, mark these routines as EXPORT_SYMBOL_GPL as
well.

Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 mm/hmm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dan Williams July 6, 2018, 11:53 p.m. UTC | #1
On Mon, Jun 18, 2018 at 11:05 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> The routines hmm_devmem_add(), and hmm_devmem_add_resource() are
> now wrappers around the functionality provided by devm_memremap_pages() to
> inject a dev_pagemap instance and hook page-idle events. The
> devm_memremap_pages() interface is base infrastructure for HMM which has
> more and deeper ties into the kernel memory management implementation
> than base ZONE_DEVICE.
>
> Originally, the HMM page structure creation routines copied the
> devm_memremap_pages() code and reused ZONE_DEVICE. A cleanup to unify
> the implementations was discussed during the initial review:
> http://lkml.iu.edu/hypermail/linux/kernel/1701.2/00812.html
>
> Given that devm_memremap_pages() is marked EXPORT_SYMBOL_GPL by its
> authors and the hmm_devmem_{add,add_resource} routines are simple
> wrappers around that base, mark these routines as EXPORT_SYMBOL_GPL as
> well.
>
> Cc: "Jérôme Glisse" <jglisse@redhat.com>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Currently OpenAFS is blocked from compiling with the 4.18 series due
to the current state of put_page() inadvertently pulling in GPL-only
symbols. This series, "PATCH v3 0/8] mm: Rework hmm to use
devm_memremap_pages and other fixes" corrects that situation and
corrects HMM's usage of EXPORT_SYMBOL_GPL.

If HMM wants to export functionality to out-of-tree proprietary
drivers it should do so without consuming GPL-only exports, or
consuming internal-only public functions in its exports.

In addition to duplicating devm_memremap_pages(), that should have
been EXPORT_SYMBOL_GPL from the beginning, it is also exporting /
consuming these GPL-only symbols via HMM's EXPORT_SYMBOL entry points.

    mmu_notifier_unregister_no_release
    percpu_ref
    region_intersects
    __class_create

Those entry points also consume / export functionality that is
currently not exported to any other driver.

    alloc_pages_vma
    walk_page_range

Andrew, please consider applying this v3 series to fix this up (let me
know if you need a resend).
Andrew Morton July 10, 2018, 12:34 a.m. UTC | #2
On Fri, 6 Jul 2018 16:53:11 -0700 Dan Williams <dan.j.williams@intel.com> wrote:

> On Mon, Jun 18, 2018 at 11:05 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > The routines hmm_devmem_add(), and hmm_devmem_add_resource() are
> > now wrappers around the functionality provided by devm_memremap_pages() to
> > inject a dev_pagemap instance and hook page-idle events. The
> > devm_memremap_pages() interface is base infrastructure for HMM which has
> > more and deeper ties into the kernel memory management implementation
> > than base ZONE_DEVICE.
> >
> > Originally, the HMM page structure creation routines copied the
> > devm_memremap_pages() code and reused ZONE_DEVICE. A cleanup to unify
> > the implementations was discussed during the initial review:
> > http://lkml.iu.edu/hypermail/linux/kernel/1701.2/00812.html
> >
> > Given that devm_memremap_pages() is marked EXPORT_SYMBOL_GPL by its
> > authors and the hmm_devmem_{add,add_resource} routines are simple
> > wrappers around that base, mark these routines as EXPORT_SYMBOL_GPL as
> > well.
> >
> > Cc: "Jérôme Glisse" <jglisse@redhat.com>
> > Cc: Logan Gunthorpe <logang@deltatee.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Currently OpenAFS is blocked from compiling with the 4.18 series due
> to the current state of put_page() inadvertently pulling in GPL-only
> symbols. This series, "PATCH v3 0/8] mm: Rework hmm to use
> devm_memremap_pages and other fixes" corrects that situation and
> corrects HMM's usage of EXPORT_SYMBOL_GPL.
> 
> If HMM wants to export functionality to out-of-tree proprietary
> drivers it should do so without consuming GPL-only exports, or
> consuming internal-only public functions in its exports.
> 
> In addition to duplicating devm_memremap_pages(), that should have
> been EXPORT_SYMBOL_GPL from the beginning, it is also exporting /
> consuming these GPL-only symbols via HMM's EXPORT_SYMBOL entry points.
> 
>     mmu_notifier_unregister_no_release
>     percpu_ref
>     region_intersects
>     __class_create
> 
> Those entry points also consume / export functionality that is
> currently not exported to any other driver.
> 
>     alloc_pages_vma
>     walk_page_range
> 
> Andrew, please consider applying this v3 series to fix this up (let me
> know if you need a resend).

A resend would be good.  And include the above info in the changelog.

I can't say I'm terribly happy with the HMM situation.  I was under the
impression that a significant number of significant in-tree drivers
would be using HMM but I've heard nothing since, apart from ongoing
nouveau work, which will be perfectly happy with GPL-only exports.

So yes, we should revisit the licensing situation and, if only nouveau
will be using HMM we should revisit HMM's overall usefulness.
Jerome Glisse July 10, 2018, 5:11 p.m. UTC | #3
On Mon, Jul 09, 2018 at 05:34:17PM -0700, Andrew Morton wrote:
> On Fri, 6 Jul 2018 16:53:11 -0700 Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > On Mon, Jun 18, 2018 at 11:05 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > > The routines hmm_devmem_add(), and hmm_devmem_add_resource() are
> > > now wrappers around the functionality provided by devm_memremap_pages() to
> > > inject a dev_pagemap instance and hook page-idle events. The
> > > devm_memremap_pages() interface is base infrastructure for HMM which has
> > > more and deeper ties into the kernel memory management implementation
> > > than base ZONE_DEVICE.
> > >
> > > Originally, the HMM page structure creation routines copied the
> > > devm_memremap_pages() code and reused ZONE_DEVICE. A cleanup to unify
> > > the implementations was discussed during the initial review:
> > > http://lkml.iu.edu/hypermail/linux/kernel/1701.2/00812.html
> > >
> > > Given that devm_memremap_pages() is marked EXPORT_SYMBOL_GPL by its
> > > authors and the hmm_devmem_{add,add_resource} routines are simple
> > > wrappers around that base, mark these routines as EXPORT_SYMBOL_GPL as
> > > well.
> > >
> > > Cc: "Jérôme Glisse" <jglisse@redhat.com>
> > > Cc: Logan Gunthorpe <logang@deltatee.com>
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > 
> > Currently OpenAFS is blocked from compiling with the 4.18 series due
> > to the current state of put_page() inadvertently pulling in GPL-only
> > symbols. This series, "PATCH v3 0/8] mm: Rework hmm to use
> > devm_memremap_pages and other fixes" corrects that situation and
> > corrects HMM's usage of EXPORT_SYMBOL_GPL.
> > 
> > If HMM wants to export functionality to out-of-tree proprietary
> > drivers it should do so without consuming GPL-only exports, or
> > consuming internal-only public functions in its exports.
> > 
> > In addition to duplicating devm_memremap_pages(), that should have
> > been EXPORT_SYMBOL_GPL from the beginning, it is also exporting /
> > consuming these GPL-only symbols via HMM's EXPORT_SYMBOL entry points.
> > 
> >     mmu_notifier_unregister_no_release
> >     percpu_ref
> >     region_intersects
> >     __class_create
> > 
> > Those entry points also consume / export functionality that is
> > currently not exported to any other driver.
> > 
> >     alloc_pages_vma
> >     walk_page_range
> > 
> > Andrew, please consider applying this v3 series to fix this up (let me
> > know if you need a resend).
> 
> A resend would be good.  And include the above info in the changelog.
> 
> I can't say I'm terribly happy with the HMM situation.  I was under the
> impression that a significant number of significant in-tree drivers
> would be using HMM but I've heard nothing since, apart from ongoing
> nouveau work, which will be perfectly happy with GPL-only exports.
> 
> So yes, we should revisit the licensing situation and, if only nouveau
> will be using HMM we should revisit HMM's overall usefulness.

So right now i am working on finishing another version of nouveau
patchset. Then i will be working on radeon driver, then on Intel.
I also have been in talk with Mellanox to bring back to life my
mlx5 patchset which converted ODP to use HMM. So this is also on
the radar. AMD GPU will come next.


The nouveau patchset is taking so long because nouveau have under
gone massive rewrite of how it manages channel (commands queue) and
memory. Which was a pre-requisite for doing HMM. This rework has
started going upstream since 4.14, piece by piece and it is still
not finish in 4.18. So work have been going steadily, if people
wants i can point to all the patches.

As this is the DRM subsystem we also need open source userspaca and
again we have been working on this since last year and this takes
time to. Lot of work have been done. I understand that it is not
necessarily obvious to people who do not follow mesa, dri-devel or
nouveau mailing list.

I am sorry this is taking so long but resources to work on this are
scarce. Yet this is important work as new standard develop inside the
C++ committee (everybody love C++ here right ;)) and in other high
level language will rely on features HMM provides to those drivers.

Cheers,
Jérôme
diff mbox

Patch

diff --git a/mm/hmm.c b/mm/hmm.c
index b019d67a610e..481a7a5f6f46 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -1054,7 +1054,7 @@  struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
 		return result;
 	return devmem;
 }
-EXPORT_SYMBOL(hmm_devmem_add);
+EXPORT_SYMBOL_GPL(hmm_devmem_add);
 
 struct hmm_devmem *hmm_devmem_add_resource(const struct hmm_devmem_ops *ops,
 					   struct device *device,
@@ -1108,7 +1108,7 @@  struct hmm_devmem *hmm_devmem_add_resource(const struct hmm_devmem_ops *ops,
 		return result;
 	return devmem;
 }
-EXPORT_SYMBOL(hmm_devmem_add_resource);
+EXPORT_SYMBOL_GPL(hmm_devmem_add_resource);
 
 /*
  * A device driver that wants to handle multiple devices memory through a