linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"Christoph Hellwig" <hch@lst.de>, "Linux MM" <linux-mm@kvack.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 1/7] mm, devm_memremap_pages: Mark devm_memremap_pages() EXPORT_SYMBOL_GPL
Date: Wed, 17 Oct 2018 09:30:58 -0700	[thread overview]
Message-ID: <CAPcyv4hMFQYekvZWMzKYckuVLSGd3GizRtoDudFBQj5bfxD3Mw@mail.gmail.com> (raw)
In-Reply-To: <20181017081753.GG18839@dhcp22.suse.cz>

On Wed, Oct 17, 2018 at 1:18 AM Michal Hocko <mhocko@kernel.org> wrote:
>
> On Fri 12-10-18 10:49:37, Dan Williams wrote:
> > devm_memremap_pages() is a facility that can create struct page entries
> > for any arbitrary range and give drivers the ability to subvert core
> > aspects of page management.
> >
> > Specifically the facility is tightly integrated with the kernel's memory
> > hotplug functionality. It injects an altmap argument deep into the
> > architecture specific vmemmap implementation to allow allocating from
> > specific reserved pages, and it has Linux specific assumptions about
> > page structure reference counting relative to get_user_pages() and
> > get_user_pages_fast(). It was an oversight and a mistake that this was
> > not marked EXPORT_SYMBOL_GPL from the outset.
>
> One thing is still not clear to me. Both devm_memremap_* and
> hmm_devmem_add essentially do the same thing AFAICS. They both allow to
> hotplug a device memory. Both rely on the hotplug code (namely
> add_pages) which itself is not exported to modules. One is GPL only
> while the later is a general export. Is this mismatch desirable?

That is resolved by the last patch in this series.

> API exported by the core hotplug is ad-hoc to say the least. Symbols
> that we actually export are GPL mostly (only try_offline_node is
> EXPORT_SYMBOL without any explanation whatsoever). So I would call it a
> general mess tweaked for specific usecases.
>
> I personally do not care about EXPORT_SYMBOL vs. EXPORT_SYMBOL_GPL
> much to be honest. I understand an argument that we do not care about
> out-of-tree modules a wee bit. I would just be worried those will find a
> way around and my experience tells me that it would be much uglier than
> what the core kernel can provide. But this seems more political than
> technical discussion.

I'm not worried about people finding a way around, I'm sure
cringe-worthy workarounds of core kernel details happen on a regular
basis in out-of-tree code. EXPORT_SYMBOL_GPL in this instance is not a
political statement, it is a statement of the rate of evolution and
depth of the api.

>
> > Again, devm_memremap_pagex() exposes and relies upon core kernel
> > internal assumptions and will continue to evolve along with 'struct
> > page', memory hotplug, and support for new memory types / topologies.
> > Only an in-kernel GPL-only driver is expected to keep up with this
> > ongoing evolution. This interface, and functionality derived from this
> > interface, is not suitable for kernel-external drivers.
>
> I do not follow this line of argumentation though. We generally do not
> care about out-of-tree modules and breaking them if the interface has to
> be updated.

Exactly right. The EXPORT_SYMBOL_GPL is there to say that this api has
deep enough ties into the core kernel to lower the confidence that the
API will stay stable from one kernel revision to the next. It's also
an api that is attracting widening and varied reuse and the long term
health of the implementation depends on being able to peer deeply into
its users and refactor the interface and the core kernel as a result.

> Also what about GPL out of tree modules?

These are precisely the modules we want upstream.

> That being said, I do not mind this patch. You and Christoph are the
> authors and therefore it is you to decide. I just find the current
> situation confusing to say the least.

Hopefully I clarified, let me know if not.

  reply	other threads:[~2018-10-17 16:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-12 17:49 [PATCH v7 0/7] mm: Merge hmm into devm_memremap_pages, mark GPL-only Dan Williams
2018-10-12 17:49 ` [PATCH v7 1/7] mm, devm_memremap_pages: Mark devm_memremap_pages() EXPORT_SYMBOL_GPL Dan Williams
2018-10-17  8:17   ` Michal Hocko
2018-10-17 16:30     ` Dan Williams [this message]
2018-10-23 17:01       ` Michal Hocko
2018-10-12 17:49 ` [PATCH v7 2/7] mm, devm_memremap_pages: Kill mapping "System RAM" support Dan Williams
2018-10-12 17:49 ` [PATCH v7 3/7] mm, devm_memremap_pages: Fix shutdown handling Dan Williams
2018-10-12 17:49 ` [PATCH v7 4/7] mm, devm_memremap_pages: Add MEMORY_DEVICE_PRIVATE support Dan Williams
2018-10-12 17:49 ` [PATCH v7 5/7] mm, hmm: Use devm semantics for hmm_devmem_{add, remove} Dan Williams
2018-10-12 17:50 ` [PATCH v7 6/7] mm, hmm: Replace hmm_devmem_pages_create() with devm_memremap_pages() Dan Williams
2018-10-12 17:50 ` [PATCH v7 7/7] mm, hmm: Mark hmm_devmem_{add, add_resource} EXPORT_SYMBOL_GPL Dan Williams
2018-10-12 18:14 ` [PATCH v7 0/7] mm: Merge hmm into devm_memremap_pages, mark GPL-only Jerome Glisse
  -- strict thread matches above, loose matches on Subject: below --
2018-10-12 17:47 Dan Williams
2018-10-12 17:47 ` [PATCH v7 1/7] mm, devm_memremap_pages: Mark devm_memremap_pages() EXPORT_SYMBOL_GPL Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPcyv4hMFQYekvZWMzKYckuVLSGd3GizRtoDudFBQj5bfxD3Mw@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=hch@lst.de \
    --cc=jglisse@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --subject='Re: [PATCH v7 1/7] mm, devm_memremap_pages: Mark devm_memremap_pages() EXPORT_SYMBOL_GPL' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).