LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jerome Glisse <jglisse@redhat.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Dave Airlie <airlied@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Logan Gunthorpe <logang@deltatee.com>,
	Christoph Hellwig <hch@lst.de>, Michal Hocko <mhocko@suse.com>,
	Linux MM <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/5] mm: rework hmm to use devm_memremap_pages
Date: Tue, 5 Jun 2018 20:08:23 -0400	[thread overview]
Message-ID: <20180606000822.GE4423@redhat.com> (raw)
In-Reply-To: <CAPcyv4gSEYdnJKd=D-_yc3M=sY0HWjYzYhh5ha-v7KA4-40dsg@mail.gmail.com>

On Tue, Jun 05, 2018 at 04:06:12PM -0700, Dan Williams wrote:
> On Tue, Jun 5, 2018 at 3:19 PM, Dave Airlie <airlied@gmail.com> wrote:
> > On 6 June 2018 at 04:48, Jerome Glisse <jglisse@redhat.com> wrote:
> >> On Tue, May 29, 2018 at 04:33:49PM -0700, Dan Williams wrote:
> >>> On Tue, May 29, 2018 at 4:00 PM, Dave Airlie <airlied@gmail.com> wrote:
> >>> > On 30 May 2018 at 08:31, Dan Williams <dan.j.williams@intel.com> wrote:

[...]

> >>> It honestly was an oversight, and as we've gone on to add deeper and
> >>> deeper ties into the mm and filesystems [1] I realized this symbol was
> >>> mis-labeled.  It would be one thing if this was just some random
> >>> kernel leaf / library function, but this capability when turned on
> >>> causes the entire kernel to be recompiled as things like the
> >>> definition of put_page() changes. It's deeply integrated with how
> >>> Linux manages memory.
> >>
> >> I am personaly on the fence on deciding GPL versus non GPL export
> >> base on subjective view of what is deeply integrated and what is
> >> not. I think one can argue that every single linux kernel function
> >> is deeply integrated within the kernel, starting with all device
> >> drivers functions. One could similarly argue that nothing is ...
> >
> > This is the point I wasn't making so well, the whole deciding on a derived
> > work from the pov of one of the works isn't really going to be how a court
> > looks at it.
> >
> > At day 0, you have a Linux kernel, and a separate Windows kernel driver,
> > clearly they are not derived works.
> >
> > You add interfaces to the Windows kernel driver and it becomes a Linux
> > kernel driver, you never ship them together, derived work only if those
> > interfaces are GPL only? or derived work only if shipped together?
> > only shipped together and GPL only? Clearly not a clearcut case here.
> >
> > The code base is 99% the same, the kernel changes an export to a GPL
> > export, the external driver hasn't changed one line of code, and it suddenly
> > becomes a derived work?
> >
> > Oversights happen, but 3 years of advertising an interface under the non-GPL
> > and changing it doesn't change whether the external driver is derived or not,
> > nor will it change anyone's legal position.
> 
> My concern is the long term health and maintainability of the Linux
> kernel. HMM exports deep Linux internals out to proprietary drivers
> with no way for folks in the wider kernel community to validate that
> the interfaces are necessary or sufficient besides "take Jerome's word
> for it". Every time I've pushed back on any HMM feature the response
> is something to the effect of, "no, out of tree drivers need this".
> HMM needs to grow upstream users and the functionality needs to be
> limited to whatever those upstream users exploit. Since there are no
> upstream users of HMM, we should delete it unless / until those users
> arrive.

The raison d'être of HMM is to isolate driver from mm internal gut and
thus provide a clear contract and API to device driver. I tried to spell
that contract in include/linux/hmm.h which i can re-formulate shortly in:
  - provide call back when CPU try to access a device page so that
    memory can be migrated back to CPU accessible page under the
    control of the device driver for device synchronization reasons
    (the whole gory mm details is still in mm/migrate.c it just does
    provide way point in the migration process so that the device
    driver can synchronize and update the hardware along the way too)
  - provide a 64bits storage inside struct page so that the device
    driver can store either pointer to its internal data structure
    or store necessary informations there while page is in use in a
    process
  - inform device driver once a page is freed (ie no longer use in a
    process address space)

This virtualy isolate device driver from the inner gut of mm and allow
mm to change as long as we can keep this contract in place. As long as
device driver only use HMM API to perform any of the above and this is
my intention to push for that and try to enforce it as strongly as i
can.

Nouveau patchset have been posted and i will post newer updated version
this month and i hope this can get upstream in 4.19 abidding by the drm
sub-system requirement of having open source userspace upstream in mesa
project too (which have been under work for last few months).

This whole thing have been a big chicken and egg nightmare with moving
pieces everywhere. I wish i was better at getting all the pieces ready
at the same time but alas i was not.

> 
> 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.

Cheers,
Jérôme

  reply	other threads:[~2018-06-06  0:08 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-21 22:35 Dan Williams
2018-05-21 22:35 ` [PATCH 1/5] mm, devm_memremap_pages: mark devm_memremap_pages() EXPORT_SYMBOL_GPL Dan Williams
2018-05-22  6:29   ` Christoph Hellwig
2018-05-21 22:35 ` [PATCH 2/5] mm, devm_memremap_pages: handle errors allocating final devres action Dan Williams
2018-05-21 23:10   ` Andrew Morton
2018-05-22  0:07     ` Dan Williams
2018-05-22 16:42       ` Logan Gunthorpe
2018-05-22 16:56         ` Dan Williams
2018-05-22 17:03           ` Logan Gunthorpe
2018-05-22 17:25             ` Dan Williams
2018-05-22 17:36               ` Logan Gunthorpe
2018-05-22  6:30   ` Christoph Hellwig
2018-05-21 22:35 ` [PATCH 3/5] mm, hmm: use devm semantics for hmm_devmem_{add, remove} Dan Williams
2018-05-22  6:30   ` Christoph Hellwig
2018-05-21 22:35 ` [PATCH 4/5] mm, hmm: replace hmm_devmem_pages_create() with devm_memremap_pages() Dan Williams
2018-05-22  6:31   ` Christoph Hellwig
2018-05-22 17:13   ` Logan Gunthorpe
2018-05-22 21:38     ` Dan Williams
2018-05-21 22:35 ` [PATCH 5/5] mm, hmm: mark hmm_devmem_{add, add_resource} EXPORT_SYMBOL_GPL Dan Williams
2018-05-22  6:32   ` Christoph Hellwig
2018-05-22 21:31     ` Andrew Morton
2018-06-05 18:24       ` Jerome Glisse
2018-05-24  0:10 ` [PATCH 0/5] mm: rework hmm to use devm_memremap_pages Jerome Glisse
2018-05-24  3:18   ` Dan Williams
2018-05-24  6:35     ` Christoph Hellwig
2018-05-29 22:22     ` Dave Airlie
2018-05-29 22:31       ` Dan Williams
2018-05-29 23:00         ` Dave Airlie
2018-05-29 23:33           ` Dan Williams
2018-06-05 18:48             ` Jerome Glisse
2018-06-05 22:19               ` Dave Airlie
2018-06-05 23:06                 ` Dan Williams
2018-06-06  0:08                   ` Jerome Glisse [this message]
2018-06-06  1:33                     ` Dan Williams
2018-06-06  7:14                       ` Christoph Hellwig
2018-06-07 14:16                       ` Jerome Glisse
2018-06-07 18:39                         ` 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=20180606000822.GE4423@redhat.com \
    --to=jglisse@redhat.com \
    --cc=airlied@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=logang@deltatee.com \
    --cc=mhocko@suse.com \
    --subject='Re: [PATCH 0/5] mm: rework hmm to use devm_memremap_pages' \
    /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).