re: [PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

re: [PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

[Mark Vitale]

On Jul 11, 2018, Dan Williams wrote:
> Changes since v3 [1]:
> * Collect Logan's reviewed-by on patch 3
> * Collect John's and Joe's tested-by on patch 8
> * Update the changelog for patch 1 and 7 to better explain the
>   EXPORT_SYMBOL_GPL rationale.
> * Update the changelog for patch 2 to clarify that it is a cleanup to
>   make the following patch-3 fix easier
>
> [1]: https://lkml.org/lkml/2018/6/19/108
>
> ---
> 
> Hi Andrew,
> 
> As requested, here is a resend of the devm_memremap_pages() fixups.
> Please consider for 4.18.

What is the status of this patchset?  OpenAFS is unable to build on
Linux 4.18 without the last patch in this set:

8/8  mm: Fix exports that inadvertently make put_page() EXPORT_SYMBOL_GPL

Will this be merged soon to linux-next, and ultimately to a Linux 4.18 rc?

Thank you,
--
Mark Vitale
mvitale@sinenomine.net
on behalf of the OpenAFS release team

Re: [PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

[Andrew Morton]

On Fri, 20 Jul 2018 14:43:14 +0000 Mark Vitale <mvitale@sinenomine.net> wrote:

> On Jul 11, 2018, Dan Williams wrote:
> > Changes since v3 [1]:
> > * Collect Logan's reviewed-by on patch 3
> > * Collect John's and Joe's tested-by on patch 8
> > * Update the changelog for patch 1 and 7 to better explain the
> >   EXPORT_SYMBOL_GPL rationale.
> > * Update the changelog for patch 2 to clarify that it is a cleanup to
> >   make the following patch-3 fix easier
> >
> > [1]: https://lkml.org/lkml/2018/6/19/108
> >
> > ---
> > 
> > Hi Andrew,
> > 
> > As requested, here is a resend of the devm_memremap_pages() fixups.
> > Please consider for 4.18.
> 
> What is the status of this patchset?  OpenAFS is unable to build on
> Linux 4.18 without the last patch in this set:
> 
> 8/8  mm: Fix exports that inadvertently make put_page() EXPORT_SYMBOL_GPL
> 
> Will this be merged soon to linux-next, and ultimately to a Linux 4.18 rc?
> 

Problem is, that patch is eighth in a series which we're waiting for
Jerome to review and the changelog starts with "Now that all producers
of dev_pagemap instances in the kernel are properly converted to
EXPORT_SYMBOL_GPL...".

Is it in fact a standalone patch?  Not sure.  I'll see what the build
system has to say about that.

And it will need a new changelog.  Such as



From: Dan Williams <dan.j.williams@intel.com>
Subject: mm: fix exports that inadvertently make put_page() EXPORT_SYMBOL_GPL

e76384884344 ("mm: introduce MEMORY_DEVICE_FS_DAX and
CONFIG_DEV_PAGEMAP_OPS") added two EXPORT_SYMBOL_GPL() symbols, but these
symbols are required by the inlined put_page(), thus accidentally making
put_page() a GPL export only.  This breaks OpenAFS (at least).

Mark them EXPORT_SYMBOL() instead.

Link: http://lkml.kernel.org/r/153128611970.2928.11310692420711601254.stgit@dwillia2-desk3.amr.corp.intel.com
Fixes: e76384884344 ("mm: introduce MEMORY_DEVICE_FS_DAX and CONFIG_DEV_PAGEMAP_OPS")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reported-by: Joe Gorse <jhgorse@gmail.com>
Reported-by: John Hubbard <jhubbard@nvidia.com>
Tested-by: Joe Gorse <jhgorse@gmail.com>
Tested-by: John Hubbard <jhubbard@nvidia.com>
Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Mark Vitale <mvitale@sinenomine.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 kernel/memremap.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN kernel/memremap.c~mm-fix-exports-that-inadvertently-make-put_page-export_symbol_gpl kernel/memremap.c
--- a/kernel/memremap.c~mm-fix-exports-that-inadvertently-make-put_page-export_symbol_gpl
+++ a/kernel/memremap.c
@@ -321,7 +321,7 @@ EXPORT_SYMBOL_GPL(get_dev_pagemap);
 
 #ifdef CONFIG_DEV_PAGEMAP_OPS
 DEFINE_STATIC_KEY_FALSE(devmap_managed_key);
-EXPORT_SYMBOL_GPL(devmap_managed_key);
+EXPORT_SYMBOL(devmap_managed_key);
 static atomic_t devmap_enable;
 
 /*
@@ -362,5 +362,5 @@ void __put_devmap_managed_page(struct pa
 	} else if (!count)
 		__put_page(page);
 }
-EXPORT_SYMBOL_GPL(__put_devmap_managed_page);
+EXPORT_SYMBOL(__put_devmap_managed_page);
 #endif /* CONFIG_DEV_PAGEMAP_OPS */
_

Re: [PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

[Jerome Glisse]

On Fri, Jul 20, 2018 at 12:51:46PM -0700, Andrew Morton wrote:
> On Fri, 20 Jul 2018 14:43:14 +0000 Mark Vitale <mvitale@sinenomine.net> wrote:
> 
> > On Jul 11, 2018, Dan Williams wrote:
> > > Changes since v3 [1]:
> > > * Collect Logan's reviewed-by on patch 3
> > > * Collect John's and Joe's tested-by on patch 8
> > > * Update the changelog for patch 1 and 7 to better explain the
> > >   EXPORT_SYMBOL_GPL rationale.
> > > * Update the changelog for patch 2 to clarify that it is a cleanup to
> > >   make the following patch-3 fix easier
> > >
> > > [1]: https://lkml.org/lkml/2018/6/19/108
> > >
> > > ---
> > > 
> > > Hi Andrew,
> > > 
> > > As requested, here is a resend of the devm_memremap_pages() fixups.
> > > Please consider for 4.18.
> > 
> > What is the status of this patchset?  OpenAFS is unable to build on
> > Linux 4.18 without the last patch in this set:
> > 
> > 8/8  mm: Fix exports that inadvertently make put_page() EXPORT_SYMBOL_GPL
> > 
> > Will this be merged soon to linux-next, and ultimately to a Linux 4.18 rc?
> > 
> 
> Problem is, that patch is eighth in a series which we're waiting for
> Jerome to review and the changelog starts with "Now that all producers
> of dev_pagemap instances in the kernel are properly converted to
> EXPORT_SYMBOL_GPL...".

I am fine with the patchset modulo GPL, i did review it in the past
but i did not formaly reply as i was opose to the GPL changes. So my
only objection is with the GPL export, everything else looks fine.

I can review once more as it has been more than a month since i last
look at this patchset. I am working with Ben on nouveau right now so
if it breaks anything for me i will fix it once we do our final
rebase before posting.

Cheers,
Jerome

Re: [PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

[Matthew Wilcox]

On Fri, Jul 20, 2018 at 03:57:47PM -0400, Jerome Glisse wrote:
> On Fri, Jul 20, 2018 at 12:51:46PM -0700, Andrew Morton wrote:
> > Problem is, that patch is eighth in a series which we're waiting for
> > Jerome to review and the changelog starts with "Now that all producers
> > of dev_pagemap instances in the kernel are properly converted to
> > EXPORT_SYMBOL_GPL...".
> 
> I am fine with the patchset modulo GPL, i did review it in the past
> but i did not formaly reply as i was opose to the GPL changes. So my
> only objection is with the GPL export, everything else looks fine.

Everyone from the mm side who's looked at these patches agrees that it
reaches far too far into the guts of the mm to be anything other than
exposing internals.  It's not credible to claim that a module written that
uses these interfaces is anything other than a derived work of the kernel.

I feel these patches should be merged over Jerome's objections.

Re: [PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

[Jerome Glisse]

On Fri, Jul 20, 2018 at 01:01:24PM -0700, Matthew Wilcox wrote:
> On Fri, Jul 20, 2018 at 03:57:47PM -0400, Jerome Glisse wrote:
> > On Fri, Jul 20, 2018 at 12:51:46PM -0700, Andrew Morton wrote:
> > > Problem is, that patch is eighth in a series which we're waiting for
> > > Jerome to review and the changelog starts with "Now that all producers
> > > of dev_pagemap instances in the kernel are properly converted to
> > > EXPORT_SYMBOL_GPL...".
> > 
> > I am fine with the patchset modulo GPL, i did review it in the past
> > but i did not formaly reply as i was opose to the GPL changes. So my
> > only objection is with the GPL export, everything else looks fine.
> 
> Everyone from the mm side who's looked at these patches agrees that it
> reaches far too far into the guts of the mm to be anything other than
> exposing internals.  It's not credible to claim that a module written that
> uses these interfaces is anything other than a derived work of the kernel.
> 
> I feel these patches should be merged over Jerome's objections.

I feel that people do not understand how far reaching this is. It means
that any new devices with memory supporting new system bus like CAPI or
CCIX will need to have a GPL driver. This is a departure of current
state of affair where we allow non GPL driver to exist.

Moreover I have argue that HMM abstract the internal mm and by doing so
allow anyone to update the mm code without having to worried about driver
which use HMM. Thus disproving that user of HMM are tie to mm internal.


Also to make thing perfectly clear i am a strong proponent of open
source and i rather have a GPL driver but at the same time i do not want
linux kernel to become second citizen because it can not support new
devices ...


Cheers,
Jerome

Re: [PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

[Dan Williams]

On Fri, Jul 20, 2018 at 1:17 PM Jerome Glisse <jglisse@redhat.com> wrote:
>
> On Fri, Jul 20, 2018 at 01:01:24PM -0700, Matthew Wilcox wrote:
> > On Fri, Jul 20, 2018 at 03:57:47PM -0400, Jerome Glisse wrote:
> > > On Fri, Jul 20, 2018 at 12:51:46PM -0700, Andrew Morton wrote:
> > > > Problem is, that patch is eighth in a series which we're waiting for
> > > > Jerome to review and the changelog starts with "Now that all producers
> > > > of dev_pagemap instances in the kernel are properly converted to
> > > > EXPORT_SYMBOL_GPL...".
> > >
> > > I am fine with the patchset modulo GPL, i did review it in the past
> > > but i did not formaly reply as i was opose to the GPL changes. So my
> > > only objection is with the GPL export, everything else looks fine.
> >
> > Everyone from the mm side who's looked at these patches agrees that it
> > reaches far too far into the guts of the mm to be anything other than
> > exposing internals.  It's not credible to claim that a module written that
> > uses these interfaces is anything other than a derived work of the kernel.
> >
> > I feel these patches should be merged over Jerome's objections.
>
> I feel that people do not understand how far reaching this is. It means
> that any new devices with memory supporting new system bus like CAPI or
> CCIX will need to have a GPL driver. This is a departure of current
> state of affair where we allow non GPL driver to exist.

Proprietary GPU driver vendors have done just fine without us adding
explicit new mechanisms for them to consume.

> Moreover I have argue that HMM abstract the internal mm and by doing so
> allow anyone to update the mm code without having to worried about driver
> which use HMM. Thus disproving that user of HMM are tie to mm internal.

No, HMM has has deployed a GPL-bypass shim into the kernel.

> Also to make thing perfectly clear i am a strong proponent of open
> source and i rather have a GPL driver but at the same time i do not want
> linux kernel to become second citizen because it can not support new
> devices ...

HMM diminishes the letter and the spirit of EXPORT_SYMBOL_GPL, it
grants access to and consumes GPL-only infrastructure written by me
and others.

[PATCH v4 0/8] mm: Rework hmm to use devm_memremap_pages and other fixes

[Dan Williams]

Changes since v3 [1]:
* Collect Logan's reviewed-by on patch 3
* Collect John's and Joe's tested-by on patch 8
* Update the changelog for patch 1 and 7 to better explain the
  EXPORT_SYMBOL_GPL rationale.
* Update the changelog for patch 2 to clarify that it is a cleanup to
  make the following patch-3 fix easier

[1]: https://lkml.org/lkml/2018/6/19/108

---

Hi Andrew,

As requested, here is a resend of the devm_memremap_pages() fixups.
Please consider for 4.18.

---

As ZONE_DEVICE continues to attract new users, it is imperative to keep
all users consolidated on devm_memremap_pages() as the interface for
create "device pages".

The devm_memremap_pages() implementation was recently reworked to make
it more generic for arbitrary users, like the proposed peer-to-peer
PCI-E enabling. HMM pre-dated this rework and opted to duplicate
devm_memremap_pages() as hmm_devmem_pages_create().

Rework hmm to be a consumer of devm_memremap_pages() directly and fix up
the licensing on the exports given the deep dependencies and exposure of
core mm internals.

With the exports of devm_memremap_pages() and hmm fixed up we can fix
the regression of inadvertently making put_page() have EXPORT_SYMBOL_GPL
dependencies, which breaks consumers like OpenAFS.

The series was tested against v4.18-rc2.

---

Dan Williams (8):
      mm, devm_memremap_pages: Mark devm_memremap_pages() EXPORT_SYMBOL_GPL
      mm, devm_memremap_pages: Kill mapping "System RAM" support
      mm, devm_memremap_pages: Fix shutdown handling
      mm, devm_memremap_pages: Add MEMORY_DEVICE_PRIVATE support
      mm, hmm: Use devm semantics for hmm_devmem_{add,remove}
      mm, hmm: Replace hmm_devmem_pages_create() with devm_memremap_pages()
      mm, hmm: Mark hmm_devmem_{add,add_resource} EXPORT_SYMBOL_GPL
      mm: Fix exports that inadvertently make put_page() EXPORT_SYMBOL_GPL


 drivers/dax/pmem.c                |   10 -
 drivers/nvdimm/pmem.c             |   18 +-
 include/linux/hmm.h               |    4 
 include/linux/memremap.h          |    7 +
 kernel/memremap.c                 |   89 +++++++----
 mm/hmm.c                          |  306 +++++--------------------------------
 tools/testing/nvdimm/test/iomap.c |   21 ++-
 7 files changed, 132 insertions(+), 323 deletions(-)