diff mbox

[5/5] mm, hmm: mark hmm_devmem_{add, add_resource} EXPORT_SYMBOL_GPL

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

Commit Message

Dan Williams May 21, 2018, 10:35 p.m. UTC
The routines hmm_devmem_add(), and hmm_devmem_add_resource() are small
wrappers around devm_memremap_pages(). The devm_memremap_pages()
interface is a subset of the hmm functionality which has more and deeper
ties into the kernel memory management implementation. It was an
oversight that these symbols were not marked EXPORT_SYMBOL_GPL from the
outset due to how they originally copied (and now reuse)
devm_memremap_pages().

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

Comments

Christoph Hellwig May 22, 2018, 6:32 a.m. UTC | #1
On Mon, May 21, 2018 at 03:35:40PM -0700, Dan Williams wrote:
> The routines hmm_devmem_add(), and hmm_devmem_add_resource() are small
> wrappers around devm_memremap_pages(). The devm_memremap_pages()
> interface is a subset of the hmm functionality which has more and deeper
> ties into the kernel memory management implementation. It was an
> oversight that these symbols were not marked EXPORT_SYMBOL_GPL from the
> outset due to how they originally copied (and now reuse)
> devm_memremap_pages().

If we end up keeping this code: absolutely.  Then again I think without
an actual user this should have never been merged, and should be removed
until one shows up.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Andrew Morton May 22, 2018, 9:31 p.m. UTC | #2
On Tue, 22 May 2018 08:32:36 +0200 Christoph Hellwig <hch@lst.de> wrote:

> On Mon, May 21, 2018 at 03:35:40PM -0700, Dan Williams wrote:
> > The routines hmm_devmem_add(), and hmm_devmem_add_resource() are small
> > wrappers around devm_memremap_pages(). The devm_memremap_pages()
> > interface is a subset of the hmm functionality which has more and deeper
> > ties into the kernel memory management implementation. It was an
> > oversight that these symbols were not marked EXPORT_SYMBOL_GPL from the
> > outset due to how they originally copied (and now reuse)
> > devm_memremap_pages().
> 
> If we end up keeping this code: absolutely.  Then again I think without
> an actual user this should have never been merged, and should be removed
> until one shows up.
> 

It wasn't simple.  Quite a lot of manufacturers were (are?) developing
quite complex driver code which utilizes hmm.  Merging hmm to give a
stable target for that development and in the expectation that those
things would be coming along was a risk and I don't think we yet know
the outcome.

Jerome, are you able to provide any updates on all of this?
Jerome Glisse June 5, 2018, 6:24 p.m. UTC | #3
On Tue, May 22, 2018 at 02:31:21PM -0700, Andrew Morton wrote:
> On Tue, 22 May 2018 08:32:36 +0200 Christoph Hellwig <hch@lst.de> wrote:
> 
> > On Mon, May 21, 2018 at 03:35:40PM -0700, Dan Williams wrote:
> > > The routines hmm_devmem_add(), and hmm_devmem_add_resource() are small
> > > wrappers around devm_memremap_pages(). The devm_memremap_pages()
> > > interface is a subset of the hmm functionality which has more and deeper
> > > ties into the kernel memory management implementation. It was an
> > > oversight that these symbols were not marked EXPORT_SYMBOL_GPL from the
> > > outset due to how they originally copied (and now reuse)
> > > devm_memremap_pages().
> > 
> > If we end up keeping this code: absolutely.  Then again I think without
> > an actual user this should have never been merged, and should be removed
> > until one shows up.
> > 
> 
> It wasn't simple.  Quite a lot of manufacturers were (are?) developing
> quite complex driver code which utilizes hmm.  Merging hmm to give a
> stable target for that development and in the expectation that those
> things would be coming along was a risk and I don't think we yet know
> the outcome.
> 
> Jerome, are you able to provide any updates on all of this?

Sorry for taking so long to reply to this, I am just back from vacation.

I posted a v1 for nouveau to use HMM back in April or early May. I want
to post a v2 soon in June. For it to get upstream it needs to fullfill
linux drm sub-system requirement which are an open source userspace for
any functionality added to GPU driver. Work for this have been going
on for a while too and userspace bits are slowly getting upstream inside
Mesa. I need to sync up to see what is still missing in Mesa.

So i won't be able to get nouveau HMM bits merge before the userspace
bits are merge too. I was hopping for 4.18 but more likely 4.19.

I know HMM have been a big chicken and egg thing and that timing for
the egg did not match the timing for the chicken :) But it is getting
there.


Also I expect more hardware and associated upstream driver to make use
of HMM but i can not comment further on that at this time because of
NDA.

Cheers,
Jérôme
diff mbox

Patch

diff --git a/mm/hmm.c b/mm/hmm.c
index a4162406067c..d9aef1266ed6 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -1072,7 +1072,7 @@  struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
 
 	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,
@@ -1131,7 +1131,7 @@  struct hmm_devmem *hmm_devmem_add_resource(const struct hmm_devmem_ops *ops,
 
 	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