[PATCH] A long explanation for a short patch

[PATCH] A long explanation for a short patch

[john.hubbard]

From: John Hubbard <jhubbard@nvidia.com>

Hi Sasha and linux-mm,

Prior to commit 309381feaee564281c3d9e90fbca8963bb7428ad, it was
possible to build MIT-licensed (non-GPL) drivers on Fedora. Fedora is
semi-unique, in that it sets CONFIG_VM_DEBUG.

Because Fedora sets CONFIG_VM_DEBUG, they end up pulling in
dump_page(), via VM_BUG_ON_PAGE, via get_page().  As one of the
authors of NVIDIA's new, open source, "UVM-Lite" kernel module, I
originally choose to use the kernel's get_page() routine from within
nvidia_uvm_page_cache.c, because get_page() has always seemed to be
very clearly intended for use by non-GPL, driver code.

So I'm hoping that making get_page() widely accessible again will not
be too controversial. We did check with Fedora first, and they
responded (https://bugzilla.redhat.com/show_bug.cgi?id=1074710#c3)
that we should try to get upstream changed, before asking Fedora
to change.  Their reasoning seems beneficial to Linux: leaving
CONFIG_DEBUG_VM set allows Fedora to help catch mm bugs.

thanks,
John h

John Hubbard (1):
  Change mm debug routines back to EXPORT_SYMBOL

 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.9.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] Change mm debug routines back to EXPORT_SYMBOL

[john.hubbard]

From: John Hubbard <jhubbard@nvidia.com>

A new dump_page() routine was recently added, and marked
EXPORT_SYMBOL_GPL. This routine was also added to the
VM_BUG_ON_PAGE() macro, and so the end result is that non-GPL
code can no longer call get_page() and a few other routines.

This trivial patch simply changes dump_page() to be
EXPORT_SYMBOL.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3bac76a..7a92925 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6564,4 +6564,4 @@ void dump_page(struct page *page, char *reason)
 {
 	dump_page_badflags(page, reason, 0);
 }
-EXPORT_SYMBOL_GPL(dump_page);
+EXPORT_SYMBOL(dump_page);
-- 
1.9.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] A long explanation for a short patch

[Sasha Levin]

On 03/13/2014 10:30 PM, john.hubbard@gmail.com wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
> Hi Sasha and linux-mm,
>
> Prior to commit 309381feaee564281c3d9e90fbca8963bb7428ad, it was
> possible to build MIT-licensed (non-GPL) drivers on Fedora. Fedora is
> semi-unique, in that it sets CONFIG_VM_DEBUG.
>
> Because Fedora sets CONFIG_VM_DEBUG, they end up pulling in
> dump_page(), via VM_BUG_ON_PAGE, via get_page().  As one of the
> authors of NVIDIA's new, open source, "UVM-Lite" kernel module, I
> originally choose to use the kernel's get_page() routine from within
> nvidia_uvm_page_cache.c, because get_page() has always seemed to be
> very clearly intended for use by non-GPL, driver code.
>
> So I'm hoping that making get_page() widely accessible again will not
> be too controversial. We did check with Fedora first, and they
> responded (https://bugzilla.redhat.com/show_bug.cgi?id=1074710#c3)
> that we should try to get upstream changed, before asking Fedora
> to change.  Their reasoning seems beneficial to Linux: leaving
> CONFIG_DEBUG_VM set allows Fedora to help catch mm bugs.

Thanks for pointing it out. I've definitely overlooked it as a
consequence of the patch. My reasoning behind making it _GPL() was
simply that it's a new export, so it's GPL unless there's a really
good excuse to make it non-GPL.

However, dump_page() as well as the regular VM_BUG_ON() are debug
functions that access functionality which isn't essential for
non-GPL modules.

This isn't the first and only case where enabling debug options will
turn code that was previously usable under a non-GPL license into
GPL specific. For example:

  - CONFIG_LOCKDEP* will turn locks GPL-only.
  - CONFIG_DYNAMIC_DEBUG will turn module loading GPL-only.
  - CONFIG_SUNRPC_DEBUG will turn the net RPC code GPL-only.

To keep it short, my opinion is that since it doesn't break any existing
code it should be kept as _GPL(), same way it was done for various other
subsystems.

Also, I think that enabling CONFIG_DEBUG_VM for end-users is a very risky
thing to do. I agree you'll find more bugs, but you'll also hit one of the many
false-positives hidden there as well. I've reported a few of those but
in some cases it's hard to determine whether it's an actual false-positive
or a bug somewhere else. Since the assumption is that end-users won't
have CONFIG_DEBUG_VM, they don't get all the attention they deserve and
end up slipping into releases: http://www.spinics.net/lists/linux-mm/msg70368.html .

Actually, I can think of a few cases where having CONFIG_DEBUG_VM enabled would
qualify a rather simple code to a CVE status.


Thanks,
Sasha


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] A long explanation for a short patch

[John Hubbard]

On 03/13/2014 09:36 PM, Sasha Levin wrote:
> On 03/13/2014 10:30 PM, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
>>
>> Hi Sasha and linux-mm,
>>
>> Prior to commit 309381feaee564281c3d9e90fbca8963bb7428ad, it was
>> possible to build MIT-licensed (non-GPL) drivers on Fedora. Fedora is
>> semi-unique, in that it sets CONFIG_VM_DEBUG.
>>
>> Because Fedora sets CONFIG_VM_DEBUG, they end up pulling in
>> dump_page(), via VM_BUG_ON_PAGE, via get_page().  As one of the
>> authors of NVIDIA's new, open source, "UVM-Lite" kernel module, I
>> originally choose to use the kernel's get_page() routine from within
>> nvidia_uvm_page_cache.c, because get_page() has always seemed to be
>> very clearly intended for use by non-GPL, driver code.
>>
>> So I'm hoping that making get_page() widely accessible again will not
>> be too controversial. We did check with Fedora first, and they
>> responded (https://bugzilla.redhat.com/show_bug.cgi?id=1074710#c3)
>> that we should try to get upstream changed, before asking Fedora
>> to change.  Their reasoning seems beneficial to Linux: leaving
>> CONFIG_DEBUG_VM set allows Fedora to help catch mm bugs.
> 
> Thanks for pointing it out. I've definitely overlooked it as a
> consequence of the patch. My reasoning behind making it _GPL() was
> simply that it's a new export, so it's GPL unless there's a really
> good excuse to make it non-GPL.
> 
> However, dump_page() as well as the regular VM_BUG_ON() are debug
> functions that access functionality which isn't essential for
> non-GPL modules.
> 
> This isn't the first and only case where enabling debug options will
> turn code that was previously usable under a non-GPL license into
> GPL specific. For example:
> 
>   - CONFIG_LOCKDEP* will turn locks GPL-only.
>   - CONFIG_DYNAMIC_DEBUG will turn module loading GPL-only.
>   - CONFIG_SUNRPC_DEBUG will turn the net RPC code GPL-only.
> 
> To keep it short, my opinion is that since it doesn't break any existing
> code it should be kept as _GPL(), same way it was done for various other
> subsystems.
> 
> Also, I think that enabling CONFIG_DEBUG_VM for end-users is a very risky
> thing to do. I agree you'll find more bugs, but you'll also hit one of the many
> false-positives hidden there as well. I've reported a few of those but
> in some cases it's hard to determine whether it's an actual false-positive
> or a bug somewhere else. Since the assumption is that end-users won't
> have CONFIG_DEBUG_VM, they don't get all the attention they deserve and
> end up slipping into releases: http://www.spinics.net/lists/linux-mm/msg70368.html .

OK, fair enough. I'm adding Fedora's Josh Boyer to CC, in case he wants to
weigh in, but that's about as hard as I'm really willing to push here. :)

thanks so much for the quick and courteous response, btw.

> 
> Actually, I can think of a few cases where having CONFIG_DEBUG_VM enabled would
> qualify a rather simple code to a CVE status.
> 
> 
> Thanks,
> Sasha
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] A long explanation for a short patch

[Josh Boyer]

On Thu, Mar 13, 2014 at 09:47:50PM -0700, John Hubbard wrote:
> On 03/13/2014 09:36 PM, Sasha Levin wrote:
> > On 03/13/2014 10:30 PM, john.hubbard@gmail.com wrote:
> >> From: John Hubbard <jhubbard@nvidia.com>
> >>
> >> Hi Sasha and linux-mm,
> >>
> >> Prior to commit 309381feaee564281c3d9e90fbca8963bb7428ad, it was
> >> possible to build MIT-licensed (non-GPL) drivers on Fedora. Fedora is
> >> semi-unique, in that it sets CONFIG_VM_DEBUG.
> >>
> >> Because Fedora sets CONFIG_VM_DEBUG, they end up pulling in
> >> dump_page(), via VM_BUG_ON_PAGE, via get_page().  As one of the
> >> authors of NVIDIA's new, open source, "UVM-Lite" kernel module, I
> >> originally choose to use the kernel's get_page() routine from within
> >> nvidia_uvm_page_cache.c, because get_page() has always seemed to be
> >> very clearly intended for use by non-GPL, driver code.
> >>
> >> So I'm hoping that making get_page() widely accessible again will not
> >> be too controversial. We did check with Fedora first, and they
> >> responded (https://bugzilla.redhat.com/show_bug.cgi?id=1074710#c3)
> >> that we should try to get upstream changed, before asking Fedora
> >> to change.  Their reasoning seems beneficial to Linux: leaving
> >> CONFIG_DEBUG_VM set allows Fedora to help catch mm bugs.
> > 
> > Thanks for pointing it out. I've definitely overlooked it as a
> > consequence of the patch. My reasoning behind making it _GPL() was
> > simply that it's a new export, so it's GPL unless there's a really
> > good excuse to make it non-GPL.

Breaking an open-source licensed driver isn't a good excuse? :)

> > However, dump_page() as well as the regular VM_BUG_ON() are debug
> > functions that access functionality which isn't essential for
> > non-GPL modules.

Doesn't your change make it somewhat essential to the drivers calling
the functions that _are_ essential?  The dump_page function itself
wasn't previously exported at all, and I'm guessing you had to export
it because of the inline functions dragging it in and drivers using
those functions.  It seems the biggest issue stems from the previously
working inline functions now being marked as GPL-only in this case when
they clearly weren't before.

> > This isn't the first and only case where enabling debug options will
> > turn code that was previously usable under a non-GPL license into
> > GPL specific. For example:
> > 
> >   - CONFIG_LOCKDEP* will turn locks GPL-only.
> >   - CONFIG_DYNAMIC_DEBUG will turn module loading GPL-only.
> >   - CONFIG_SUNRPC_DEBUG will turn the net RPC code GPL-only.
> >
> > To keep it short, my opinion is that since it doesn't break any existing
> > code it should be kept as _GPL(), same way it was done for various other
> > subsystems.

I'm not entirely sure that's an apples to apples comparison, or that
some of those statements are even accurate.  I really don't want to get
into that debate though.

> > Also, I think that enabling CONFIG_DEBUG_VM for end-users is a very risky
> > thing to do. I agree you'll find more bugs, but you'll also hit one of the many
> > false-positives hidden there as well. I've reported a few of those but
> > in some cases it's hard to determine whether it's an actual false-positive
> > or a bug somewhere else. Since the assumption is that end-users won't
> > have CONFIG_DEBUG_VM, they don't get all the attention they deserve and
> > end up slipping into releases: http://www.spinics.net/lists/linux-mm/msg70368.html .

The last time this came up not that long ago, various MM people were
surprised we had it enabled but overall supportive.  I think Hugh and
Dave were involved in those discussions.  I'm certainly willing to
revisit having it enabled if the state of the code has changed since
then.

However, if the things wrapped in DEBUG_VM aren't getting the attention
they deserve, are causing false-positives, and generally aren't
maintained, then perhaps having a config option to enable them isn't a
great idea.  Users will toggle any and all config options that are out
there and making assumptions that they aren't going to be enabled by
users is pretty silly.  Perhaps just make them depend on modifying a
#debug define at the top of each file in question?  That way "end users"
won't easily be able to turn on broken debug code.

josh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] A long explanation for a short patch

[Sasha Levin]

On 03/14/2014 09:42 AM, Josh Boyer wrote:
> On Thu, Mar 13, 2014 at 09:47:50PM -0700, John Hubbard wrote:
>> On 03/13/2014 09:36 PM, Sasha Levin wrote:
>>> On 03/13/2014 10:30 PM, john.hubbard@gmail.com wrote:
>>>> From: John Hubbard <jhubbard@nvidia.com>
>>>>
>>>> Hi Sasha and linux-mm,
>>>>
>>>> Prior to commit 309381feaee564281c3d9e90fbca8963bb7428ad, it was
>>>> possible to build MIT-licensed (non-GPL) drivers on Fedora. Fedora is
>>>> semi-unique, in that it sets CONFIG_VM_DEBUG.
>>>>
>>>> Because Fedora sets CONFIG_VM_DEBUG, they end up pulling in
>>>> dump_page(), via VM_BUG_ON_PAGE, via get_page().  As one of the
>>>> authors of NVIDIA's new, open source, "UVM-Lite" kernel module, I
>>>> originally choose to use the kernel's get_page() routine from within
>>>> nvidia_uvm_page_cache.c, because get_page() has always seemed to be
>>>> very clearly intended for use by non-GPL, driver code.
>>>>
>>>> So I'm hoping that making get_page() widely accessible again will not
>>>> be too controversial. We did check with Fedora first, and they
>>>> responded (https://bugzilla.redhat.com/show_bug.cgi?id=1074710#c3)
>>>> that we should try to get upstream changed, before asking Fedora
>>>> to change.  Their reasoning seems beneficial to Linux: leaving
>>>> CONFIG_DEBUG_VM set allows Fedora to help catch mm bugs.
>>>
>>> Thanks for pointing it out. I've definitely overlooked it as a
>>> consequence of the patch. My reasoning behind making it _GPL() was
>>> simply that it's a new export, so it's GPL unless there's a really
>>> good excuse to make it non-GPL.
>
> Breaking an open-source licensed driver isn't a good excuse? :)

I think there's a difference between breaking existing binaries and
breaking an out of tree codebase.

In this case it's even simpler, we don't actually break any part of the code.
The breakage occurs as a result of config options, and not of code changes.

>>> However, dump_page() as well as the regular VM_BUG_ON() are debug
>>> functions that access functionality which isn't essential for
>>> non-GPL modules.
>
> Doesn't your change make it somewhat essential to the drivers calling
> the functions that _are_ essential?  The dump_page function itself
> wasn't previously exported at all, and I'm guessing you had to export
> it because of the inline functions dragging it in and drivers using
> those functions.  It seems the biggest issue stems from the previously
> working inline functions now being marked as GPL-only in this case when
> they clearly weren't before.

It makes it essential only when you enable CONFIG_DEBUG_VM, and when you
enable CONFIG_DEBUG_VM you're basically saying "I want more than the bare
minimum required for my driver", and at that point you enter GPL-land.

Previously working inline functions will keep working just the same as
before if you don't request them to be compiled with CONFIG_DEBUG_VM.

>>> This isn't the first and only case where enabling debug options will
>>> turn code that was previously usable under a non-GPL license into
>>> GPL specific. For example:
>>>
>>>    - CONFIG_LOCKDEP* will turn locks GPL-only.
>>>    - CONFIG_DYNAMIC_DEBUG will turn module loading GPL-only.
>>>    - CONFIG_SUNRPC_DEBUG will turn the net RPC code GPL-only.
>>>
>>> To keep it short, my opinion is that since it doesn't break any existing
>>> code it should be kept as _GPL(), same way it was done for various other
>>> subsystems.
>
> I'm not entirely sure that's an apples to apples comparison, or that
> some of those statements are even accurate.  I really don't want to get
> into that debate though.

Fair enough. I'm not trying to say that it's exactly the same issue, but
only that such issues occurred in the past and were dealt differently by
Fedora. From the text of the Fedora bug report on the subject:

"""
A similar conflict exists between the nvidia.ko module and kernels with
CONFIG_DEBUG_LOCK_ALLOC enabled: Fedora alpha kernels have this option
enabled by default, and Fedora beta and release kernels have it disabled.
"""

>>> Also, I think that enabling CONFIG_DEBUG_VM for end-users is a very risky
>>> thing to do. I agree you'll find more bugs, but you'll also hit one of the many
>>> false-positives hidden there as well. I've reported a few of those but
>>> in some cases it's hard to determine whether it's an actual false-positive
>>> or a bug somewhere else. Since the assumption is that end-users won't
>>> have CONFIG_DEBUG_VM, they don't get all the attention they deserve and
>>> end up slipping into releases: http://www.spinics.net/lists/linux-mm/msg70368.html .
>
> The last time this came up not that long ago, various MM people were
> surprised we had it enabled but overall supportive.  I think Hugh and
> Dave were involved in those discussions.  I'm certainly willing to
> revisit having it enabled if the state of the code has changed since
> then.
>
> However, if the things wrapped in DEBUG_VM aren't getting the attention
> they deserve, are causing false-positives, and generally aren't
> maintained, then perhaps having a config option to enable them isn't a
> great idea.  Users will toggle any and all config options that are out
> there and making assumptions that they aren't going to be enabled by
> users is pretty silly.  Perhaps just make them depend on modifying a
> #debug define at the top of each file in question?  That way "end users"
> won't easily be able to turn on broken debug code.

I didn't want to imply that DEBUG_VM issues are getting pushed aside and
ignored because "no one will see it". DEBUG_VM issues are being treated
like any other mm/ issue, but they do get placed lower on a lower priority
than issues that happen without DEBUG_VM due to lack of "manpower".

There's also no assumption that they won't get enabled, the only assumption
is that if you enable it then you know what you're doing.


Thanks,
Sasha

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] A long explanation for a short patch

[Josh Boyer]

On Fri, Mar 14, 2014 at 11:09:39AM -0400, Sasha Levin wrote:
> On 03/14/2014 09:42 AM, Josh Boyer wrote:
> >On Thu, Mar 13, 2014 at 09:47:50PM -0700, John Hubbard wrote:
> >>>To keep it short, my opinion is that since it doesn't break any existing
> >>>code it should be kept as _GPL(), same way it was done for various other
> >>>subsystems.
> >
> >I'm not entirely sure that's an apples to apples comparison, or that
> >some of those statements are even accurate.  I really don't want to get
> >into that debate though.
> 
> Fair enough. I'm not trying to say that it's exactly the same issue, but
> only that such issues occurred in the past and were dealt differently by
> Fedora. From the text of the Fedora bug report on the subject:
> 
> """
> A similar conflict exists between the nvidia.ko module and kernels with
> CONFIG_DEBUG_LOCK_ALLOC enabled: Fedora alpha kernels have this option
> enabled by default, and Fedora beta and release kernels have it disabled.
> """

Right.  We can do that with DEBUG_VM as well if needs be.  I suppose the
thing that gave me pause here is that the highlighted example was an
issue with a proprietary module whereas this one is permissively
licensed (more permissively than GPL even).  Unfortunately, there is no
EXPORT_SYMBOL_OPENSOURCE or equivalent today, which means things break
somewhat unnecessarily.

This entire issue could be avoided if the UVM-lite module had:

MODULE_LICENSE("Dual MIT/GPL")

instead of just "MIT".  John, is that something that is possible?

> >>>Also, I think that enabling CONFIG_DEBUG_VM for end-users is a very risky
> >>>thing to do. I agree you'll find more bugs, but you'll also hit one of the many
> >>>false-positives hidden there as well. I've reported a few of those but
> >>>in some cases it's hard to determine whether it's an actual false-positive
> >>>or a bug somewhere else. Since the assumption is that end-users won't
> >>>have CONFIG_DEBUG_VM, they don't get all the attention they deserve and
> >>>end up slipping into releases: http://www.spinics.net/lists/linux-mm/msg70368.html .
> >
> >The last time this came up not that long ago, various MM people were
> >surprised we had it enabled but overall supportive.  I think Hugh and
> >Dave were involved in those discussions.  I'm certainly willing to
> >revisit having it enabled if the state of the code has changed since
> >then.
> >
> >However, if the things wrapped in DEBUG_VM aren't getting the attention
> >they deserve, are causing false-positives, and generally aren't
> >maintained, then perhaps having a config option to enable them isn't a
> >great idea.  Users will toggle any and all config options that are out
> >there and making assumptions that they aren't going to be enabled by
> >users is pretty silly.  Perhaps just make them depend on modifying a
> >#debug define at the top of each file in question?  That way "end users"
> >won't easily be able to turn on broken debug code.
> 
> I didn't want to imply that DEBUG_VM issues are getting pushed aside and
> ignored because "no one will see it". DEBUG_VM issues are being treated
> like any other mm/ issue, but they do get placed lower on a lower priority
> than issues that happen without DEBUG_VM due to lack of "manpower".
> 
> There's also no assumption that they won't get enabled, the only assumption
> is that if you enable it then you know what you're doing.

Hm.  It's enabled by default in defconfigs for tegra (ARM), blackfin,
s390, sh, and tile machines.  I suppose the "end user" of those
architectures knows what they're doing.  It still might be worthwhile to
add some cautionary wording to the Kconfig help text.  Today that says:

"Enable this to turn on extended checks in the virtual-memory system
 that may impact performance."

Extended checks sound good.  Sounds like it would make things safer.
Why wouldn't I want them?

Perhaps this would be more accurate:

"Enable this to turn on debugging infrastructure used by kernel
virtual-memory developers.  This may impact performance.  If you are not
debugging the Linux kernel virtual-memory subsystem, say N."

josh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] A long explanation for a short patch

[Andrew Morton]

On Fri, 14 Mar 2014 12:53:32 -0400 Josh Boyer <jwboyer@redhat.com> wrote:

> I suppose the
> thing that gave me pause here is that the highlighted example was an
> issue with a proprietary module whereas this one is permissively
> licensed (more permissively than GPL even).

Doesn't really matter much.

a) things used to work, but 309381feaee564 broke it, unintentionally.

b) modules which work OK with CONFIG_DEBUG_VM=n will break with
   CONFIG_DEBUG_VM=y, which makes no sense.


I queued the patch for 3.15-rc1 with a tweaked changelog:


From: John Hubbard <jhubbard@nvidia.com>
Subject: mm/page_alloc.c: change mm debug routines back to EXPORT_SYMBOL

A new dump_page() routine was recently added, and marked
EXPORT_SYMBOL_GPL.  dump_page() was also added to the VM_BUG_ON_PAGE()
macro, and so the end result is that non-GPL code can no longer call
get_page() and a few other routines.

This only happens if the kernel was compiled with CONFIG_DEBUG_VM.

Change dump_page() to be EXPORT_SYMBOL.

Longer explanation:

Prior to 309381feaee564 ("mm: dump page when hitting a VM_BUG_ON using
VM_BUG_ON_PAGE") , it was possible to build MIT-licensed (non-GPL) drivers
on Fedora.  Fedora is semi-unique, in that it sets CONFIG_VM_DEBUG.

Because Fedora sets CONFIG_VM_DEBUG, they end up pulling in dump_page(),
via VM_BUG_ON_PAGE, via get_page().  As one of the authors of NVIDIA's
new, open source, "UVM-Lite" kernel module, I originally choose to use the
kernel's get_page() routine from within nvidia_uvm_page_cache.c, because
get_page() has always seemed to be very clearly intended for use by
non-GPL, driver code.

So I'm hoping that making get_page() widely accessible again will not be
too controversial.  We did check with Fedora first, and they responded
(https://bugzilla.redhat.com/show_bug.cgi?id=1074710#c3) that we should
try to get upstream changed, before asking Fedora to change.  Their
reasoning seems beneficial to Linux: leaving CONFIG_DEBUG_VM set allows
Fedora to help catch mm bugs.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Josh Boyer <jwboyer@redhat.com>

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/page_alloc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN mm/page_alloc.c~mm-page_allocc-change-mm-debug-routines-back-to-export_symbol mm/page_alloc.c
--- a/mm/page_alloc.c~mm-page_allocc-change-mm-debug-routines-back-to-export_symbol
+++ a/mm/page_alloc.c
@@ -6566,4 +6566,4 @@ void dump_page(struct page *page, const
 {
 	dump_page_badflags(page, reason, 0);
 }
-EXPORT_SYMBOL_GPL(dump_page);
+EXPORT_SYMBOL(dump_page);
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>