[PATCH] mm: Change global memory state symbols to GPL-only

[PATCH] mm: Change global memory state symbols to GPL-only

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

Proprietary modules should not be able to touch vm_stat or participate
in shrinking.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 mm/vmscan.c | 4 ++--
 mm/vmstat.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 8286938..e6e7449 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -247,7 +247,7 @@ int register_shrinker(struct shrinker *shrinker)
 	up_write(&shrinker_rwsem);
 	return 0;
 }
-EXPORT_SYMBOL(register_shrinker);
+EXPORT_SYMBOL_GPL(register_shrinker);
 
 /*
  * Remove one
@@ -259,7 +259,7 @@ void unregister_shrinker(struct shrinker *shrinker)
 	up_write(&shrinker_rwsem);
 	kfree(shrinker->nr_deferred);
 }
-EXPORT_SYMBOL(unregister_shrinker);
+EXPORT_SYMBOL_GPL(unregister_shrinker);
 
 #define SHRINK_BATCH 128
 
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4f5cd97..6d3f8f4 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -87,7 +87,7 @@ void vm_events_fold_cpu(int cpu)
  * vm_stat contains the global counters
  */
 atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS] __cacheline_aligned_in_smp;
-EXPORT_SYMBOL(vm_stat);
+EXPORT_SYMBOL_GPL(vm_stat);
 
 #ifdef CONFIG_SMP
 
-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Michal Hocko]

On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> Proprietary modules should not be able to touch vm_stat or participate
> in shrinking.

How does the external and !GPL fs does slab reclaim? Those are essential
for the proper memory balancing.

You are probably right about vm_stat though. Those counters should be
out of those modules.

> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  mm/vmscan.c | 4 ++--
>  mm/vmstat.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 8286938..e6e7449 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -247,7 +247,7 @@ int register_shrinker(struct shrinker *shrinker)
>  	up_write(&shrinker_rwsem);
>  	return 0;
>  }
> -EXPORT_SYMBOL(register_shrinker);
> +EXPORT_SYMBOL_GPL(register_shrinker);
>  
>  /*
>   * Remove one
> @@ -259,7 +259,7 @@ void unregister_shrinker(struct shrinker *shrinker)
>  	up_write(&shrinker_rwsem);
>  	kfree(shrinker->nr_deferred);
>  }
> -EXPORT_SYMBOL(unregister_shrinker);
> +EXPORT_SYMBOL_GPL(unregister_shrinker);
>  
>  #define SHRINK_BATCH 128
>  
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 4f5cd97..6d3f8f4 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -87,7 +87,7 @@ void vm_events_fold_cpu(int cpu)
>   * vm_stat contains the global counters
>   */
>  atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS] __cacheline_aligned_in_smp;
> -EXPORT_SYMBOL(vm_stat);
> +EXPORT_SYMBOL_GPL(vm_stat);
>  
>  #ifdef CONFIG_SMP
>  
> -- 
> Ben Hutchings
> [W]e found...that it wasn't as easy to get programs right as we had thought.
> ... I realized that a large part of my life from then on was going to be spent
> in finding mistakes in my own programs. - Maurice Wilkes, 1949
> 



-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2344 bytes --]

On Mon, 2015-08-17 at 15:54 +0200, Michal Hocko wrote:
> On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> > Proprietary modules should not be able to touch vm_stat or participate
> > in shrinking.
> 
> How does the external and !GPL fs does slab reclaim? Those are essential
> for the proper memory balancing.

If they know how to do shrinking on Linux then they are probably
derivative works of Linux.

Ben.

> You are probably right about vm_stat though. Those counters should be
> out of those modules.
> 
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> >  mm/vmscan.c | 4 ++--
> >  mm/vmstat.c | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 8286938..e6e7449 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -247,7 +247,7 @@ int register_shrinker(struct shrinker *shrinker)
> >  > > 	> > up_write(&shrinker_rwsem);
> >  > > 	> > return 0;
> >  }
> > -EXPORT_SYMBOL(register_shrinker);
> > +EXPORT_SYMBOL_GPL(register_shrinker);
> >  
> >  /*
> >   * Remove one
> > @@ -259,7 +259,7 @@ void unregister_shrinker(struct shrinker *shrinker)
> >  > > 	> > up_write(&shrinker_rwsem);
> >  > > 	> > kfree(shrinker->nr_deferred);
> >  }
> > -EXPORT_SYMBOL(unregister_shrinker);
> > +EXPORT_SYMBOL_GPL(unregister_shrinker);
> >  
> >  #define SHRINK_BATCH 128
> >  
> > diff --git a/mm/vmstat.c b/mm/vmstat.c
> > index 4f5cd97..6d3f8f4 100644
> > --- a/mm/vmstat.c
> > +++ b/mm/vmstat.c
> > @@ -87,7 +87,7 @@ void vm_events_fold_cpu(int cpu)
> >   * vm_stat contains the global counters
> >   */
> >  atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS] __cacheline_aligned_in_smp;
> > -EXPORT_SYMBOL(vm_stat);
> > +EXPORT_SYMBOL_GPL(vm_stat);
> >  
> >  #ifdef CONFIG_SMP
> >  
> > -- 
> > Ben Hutchings
> > [W]e found...that it wasn't as easy to get programs right as we had thought.
> > ... I realized that a large part of my life from then on was going to be spent
> > in finding mistakes in my own programs. - Maurice Wilkes, 1949
> > 
> 
> 
> 
-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Michal Hocko]

On Mon 17-08-15 16:56:32, Ben Hutchings wrote:
> On Mon, 2015-08-17 at 15:54 +0200, Michal Hocko wrote:
> > On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> > > Proprietary modules should not be able to touch vm_stat or participate
> > > in shrinking.
> > 
> > How does the external and !GPL fs does slab reclaim? Those are essential
> > for the proper memory balancing.
> 
> If they know how to do shrinking on Linux then they are probably
> derivative works of Linux.

I am not sure I understand. They are shrinking their internal cached
objects and that is hardly a derivative work. The shrinker API is only
meant to let them know _when_ this should happen and the interface is
a pretty much simple callback API.

I do not want to defend a proprietary code here but this sounds like an
obstruction for those modules which will lead into a worse code in the
end because they should somehow manage the cache and it is much better
when the core (MM) tells them when it makes sense rather than external
heuristics.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]

On Mon, 2015-08-17 at 17:11 +0200, Michal Hocko wrote:
> On Mon 17-08-15 16:56:32, Ben Hutchings wrote:
> > On Mon, 2015-08-17 at 15:54 +0200, Michal Hocko wrote:
> > > On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> > > > Proprietary modules should not be able to touch vm_stat or participate
> > > > in shrinking.
> > > 
> > > How does the external and !GPL fs does slab reclaim? Those are essential
> > > for the proper memory balancing.
> > 
> > If they know how to do shrinking on Linux then they are probably
> > derivative works of Linux.
> 
> I am not sure I understand. They are shrinking their internal cached
> objects and that is hardly a derivative work. The shrinker API is only
> meant to let them know _when_ this should happen and the interface is
> a pretty much simple callback API.

It is a Linux-specific API and I don't think other kernels provide
something similar to loadable modules.  It enables a module to turn a
large part of the system RAM into a cache and have the MM effectively
tell it the correct size of that cache, thus tightly integrating with
global memory management.

It seemed to me that this met the test for 'should this be
EXPORT_SYMBOL_GPL'.

> I do not want to defend a proprietary code here but this sounds like an
> obstruction for those modules which will lead into a worse code in the
> end because they should somehow manage the cache and it is much better
> when the core (MM) tells them when it makes sense rather than external
> heuristics.

Yes, that's the idea, proprietary code should not be helped in this
way.

Ben.

-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Richard Yao]

I seem to have botched the reply headers, so I am resending this. I use mutt
fairly infrequently, but I am making an effort to use it more it more.

On Tue, Sep 01, 2015 at 02:21:43PM +0100, Ben Hutchings wrote:
> On Tue, 2015-09-01 at 01:24 +0000, Richard Yao wrote:
> > On Mon, 2015-08-17 09:56:55 -0700, Ben Hutchings wrote:
> > > On Mon, 2015-08-17 at 17:11 +0200, Michal Hocko wrote:
> > > > On Mon 17-08-15 16:56:32, Ben Hutchings wrote:
> > > > > On Mon, 2015-08-17 at 15:54 +0200, Michal Hocko wrote:
> > > > > > On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> > > > > > > Proprietary modules should not be able to touch vm_stat or 
> > > > > > > participate
> > > > > > > in shrinking.
> > > > > > 
> > > > > > How does the external and !GPL fs does slab reclaim? Those 
> > > > > > are essential
> > > > > > for the proper memory balancing.
> > > > > 
> > > > > If they know how to do shrinking on Linux then they are 
> > > > > probably
> > > > > derivative works of Linux.
> > > > 
> > > > I am not sure I understand. They are shrinking their internal 
> > > > cached
> > > > objects and that is hardly a derivative work. The shrinker API is 
> > > > only
> > > > meant to let them know _when_ this should happen and the 
> > > > interface is
> > > > a pretty much simple callback API.
> > > 
> > > It is a Linux-specific API and I don't think other kernels provide
> > > something similar to loadable modules.  It enables a module to turn 
> > > a
> > > large part of the system RAM into a cache and have the MM 
> > > effectively
> > > tell it the correct size of that cache, thus tightly integrating 
> > > with
> > > global memory management.
> > 
> > The idea of providing third party drivers with the hooks that they 
> > need to
> > respond to memory pressure is by no means Linux-specific. In 
> > OpenSolaris, there
> > are counters for drivers to keep track of memory usage and try to 
> > stay ahead of
> > it, which works because there is no direct reclaim. There are also 
> > callbacks
> > for the SLAB code to defragment the caches used by the drivers. 
> 
> 
> So you confirm that the Linux and Solaris APIs for this are quite
> different.

They both have VFS implementations, but the designs are different. According to
attorneys, adapting code to port a filesystem driver from a foreign VFS to the
Linux VFS to make a port is insufficient to form a derived work of Linux. All
kernels have memory management and preferred ways of dealing with pressure.
Using the preferred way of dealing with it should be similarly insufficient to
form a derived work of Linux.

> > Giving non-GPL
> > drivers to respond to memory pressure seems reasonable.
> 
> I don't see why.

It is an existing function and it is needs to be avaliable to allow third party
drivers to be good citizens inside the kernel.

I do not know all of the filesystems whose memory management will become worse
from this, but I know that ZFS is one. Lustre is another because it hooks into
ZFS. I am told that the OpenAFS client uses this too..

> > > Yes, that's the idea, proprietary code should not be helped in this
> > > way.
> > 
> > The ZFSOnLinux kernel driver uses these symbols. It is fully open 
> > source and
> > not proprietary.
> [...]
> 
> It's not GPL-compatible, which in terms of Linux kernel modules is no
> better than proprietary.

Any code under a F/OSS license follows the OSD, which means that it is under a
better license than proprietary code:

http://opensource.org/definition

Rather than sabotaging working code (especially working F/OSS code), maybe it
would be better to ask why it being used instead of code under your preferred
license and invest energy into developing better alternatives under licenses
that you prefer. If someone makes something better, people would use it and
developers will flock to it. Artificially damaging competition does not help
anyone.

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

On Tue, 2015-09-01 at 01:24 +0000, Richard Yao wrote:
> On Mon, 2015-08-17 09:56:55 -0700, Ben Hutchings wrote:
> > On Mon, 2015-08-17 at 17:11 +0200, Michal Hocko wrote:
> > > On Mon 17-08-15 16:56:32, Ben Hutchings wrote:
> > > > On Mon, 2015-08-17 at 15:54 +0200, Michal Hocko wrote:
> > > > > On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> > > > > > Proprietary modules should not be able to touch vm_stat or 
> > > > > > participate
> > > > > > in shrinking.
> > > > > 
> > > > > How does the external and !GPL fs does slab reclaim? Those 
> > > > > are essential
> > > > > for the proper memory balancing.
> > > > 
> > > > If they know how to do shrinking on Linux then they are 
> > > > probably
> > > > derivative works of Linux.
> > > 
> > > I am not sure I understand. They are shrinking their internal 
> > > cached
> > > objects and that is hardly a derivative work. The shrinker API is 
> > > only
> > > meant to let them know _when_ this should happen and the 
> > > interface is
> > > a pretty much simple callback API.
> > 
> > It is a Linux-specific API and I don't think other kernels provide
> > something similar to loadable modules.  It enables a module to turn 
> > a
> > large part of the system RAM into a cache and have the MM 
> > effectively
> > tell it the correct size of that cache, thus tightly integrating 
> > with
> > global memory management.
> 
> The idea of providing third party drivers with the hooks that they 
> need to
> respond to memory pressure is by no means Linux-specific. In 
> OpenSolaris, there
> are counters for drivers to keep track of memory usage and try to 
> stay ahead of
> it, which works because there is no direct reclaim. There are also 
> callbacks
> for the SLAB code to defragment the caches used by the drivers. 


So you confirm that the Linux and Solaris APIs for this are quite
different.

> Giving non-GPL
> drivers to respond to memory pressure seems reasonable.

I don't see why.

[...]
> > Yes, that's the idea, proprietary code should not be helped in this
> > way.
> 
> The ZFSOnLinux kernel driver uses these symbols. It is fully open 
> source and
> not proprietary.
[...]

It's not GPL-compatible, which in terms of Linux kernel modules is no
better than proprietary.

Ben.

-- 
Ben Hutchings
The first rule of tautology club is the first rule of tautology club.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Dave Chinner]

On Tue, Sep 01, 2015 at 01:24:53AM +0000, Richard Yao wrote:
> On Mon, 2015-08-17 09:56:55 -0700, Ben Hutchings wrote:
> > On Mon, 2015-08-17 at 17:11 +0200, Michal Hocko wrote:
> > > On Mon 17-08-15 16:56:32, Ben Hutchings wrote:
> > > > On Mon, 2015-08-17 at 15:54 +0200, Michal Hocko wrote:
> > > > > On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> > > > > > Proprietary modules should not be able to touch vm_stat or particip=
> > ate
> > > > > > in shrinking.
> > > > >=20
> > > > > How does the external and !GPL fs does slab reclaim? Those are essent=
> > ial
> > > > > for the proper memory balancing.
> > > >=20
> > > > If they know how to do shrinking on Linux then they are probably
> > > > derivative works of Linux.
> > >=20
> > > I am not sure I understand. They are shrinking their internal cached
> > > objects and that is hardly a derivative work. The shrinker API is only
> > > meant to let them know _when_ this should happen and the interface is
> > > a pretty much simple callback API.
> > 
> > It is a Linux-specific API and I don't think other kernels provide
> > something similar to loadable modules.  It enables a module to turn a
> > large part of the system RAM into a cache and have the MM effectively
> > tell it the correct size of that cache, thus tightly integrating with
> > global memory management.
> 
> The idea of providing third party drivers with the hooks that they need to
> respond to memory pressure is by no means Linux-specific. In OpenSolaris, there
> are counters for drivers to keep track of memory usage and try to stay ahead of
> it, which works because there is no direct reclaim. There are also callbacks
> for the SLAB code to defragment the caches used by the drivers. Giving non-GPL
> drivers to respond to memory pressure seems reasonable. Given how long this has
> been the csae, it seems strange to change it now.

Especially considering that certain shrinker implementations in the
Linux tree can be used to directly refute the "derived work"
arguments given above.

Yeah, I'm about to ramble on about XFS history again. ;)

When XFS was ported to Linux, all the "memory shakers" that XFS used
on Irix were either removed or neutered because Linux didn't have
any infrastructure to drive them.  For example, have a look at the
1996 commit to Irix that introduced XFS quota support.

http://oss.sgi.com/cgi-bin/gitweb.cgi?p=archive/xfs-import.git;a=blob;f=fs/xfs/xfs_qm.c;hb=b5a7144e5bd96fa98db6118d040dabad896ba785

Search for xfs_qm_shake() and xfs_qm_shake_freelist().
xfs_qm_shake() is the registered shaker callback that came from the
mm subsystem when memory was low. It used a priority level rather
than a count of of objects to shrink, but otherwise it's function is
identical to a Linux shrinker callback but was written years before
an XFS linux port was even thought of.

Now fast forward to the introduction of the rudimentary shrinker
infrastructure to Linux back in 2002, and we have this XFS commit:

http://oss.sgi.com/cgi-bin/gitweb.cgi?p=archive/xfs-import.git;a=commitdiff;h=27a25ec7d0cb324a0129d609edef40a7468a510f

Which connected the XFS quota shaker up to the brand spanking new
Linux shrinker infrastructure. Check the code that was being run by
this linux shrinker - it's clearly the same freelist/shaker code
that was originally committed to Irix.

The XFS quota shrinker code has change in the 13 years since then,
so it's roots are no longer quite so obvious. That, however, doesn't
mean we have forgotten where the code originally came from, nor have
we forgotten that kernel-based, subsystem-independent memory reclaim
callbacks had existed for many, many years in other operating
systems before Linux re-invented them.

So, IMO, a shrinker implementation in a kernel module does not
automatically make the kernel module a derived work of the kernel.
The concept is common across many operating systems, and the
kernel APIs are sufficiently similar in function and specification
that porting shrinkers between different operating systems is a
trivial effort....

Just my 2c worth.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] mm: Change global memory state symbols to GPL-only

[Richard Yao]

On Mon, 2015-08-17 09:56:55 -0700, Ben Hutchings wrote:
> On Mon, 2015-08-17 at 17:11 +0200, Michal Hocko wrote:
> > On Mon 17-08-15 16:56:32, Ben Hutchings wrote:
> > > On Mon, 2015-08-17 at 15:54 +0200, Michal Hocko wrote:
> > > > On Sun 16-08-15 01:42:27, Ben Hutchings wrote:
> > > > > Proprietary modules should not be able to touch vm_stat or particip=
> ate
> > > > > in shrinking.
> > > >=20
> > > > How does the external and !GPL fs does slab reclaim? Those are essent=
> ial
> > > > for the proper memory balancing.
> > >=20
> > > If they know how to do shrinking on Linux then they are probably
> > > derivative works of Linux.
> >=20
> > I am not sure I understand. They are shrinking their internal cached
> > objects and that is hardly a derivative work. The shrinker API is only
> > meant to let them know _when_ this should happen and the interface is
> > a pretty much simple callback API.
> 
> It is a Linux-specific API and I don't think other kernels provide
> something similar to loadable modules.  It enables a module to turn a
> large part of the system RAM into a cache and have the MM effectively
> tell it the correct size of that cache, thus tightly integrating with
> global memory management.

The idea of providing third party drivers with the hooks that they need to
respond to memory pressure is by no means Linux-specific. In OpenSolaris, there
are counters for drivers to keep track of memory usage and try to stay ahead of
it, which works because there is no direct reclaim. There are also callbacks
for the SLAB code to defragment the caches used by the drivers. Giving non-GPL
drivers to respond to memory pressure seems reasonable. Given how long this has
been the csae, it seems strange to change it now.

Every lawyer with whom I have spoken about ports of drivers from other
platforms has informed me tht they are not derived works of Linux. I cannot
imagine a simple callback to free memory on demand would change that.

> It seemed to me that this met the test for 'should this be
> EXPORT_SYMBOL_GPL'.
> 
> > I do not want to defend a proprietary code here but this sounds like an
> > obstruction for those modules which will lead into a worse code in the
> > end because they should somehow manage the cache and it is much better
> > when the core (MM) tells them when it makes sense rather than external
> > heuristics.
> 
> Yes, that's the idea, proprietary code should not be helped in this
> way.

The ZFSOnLinux kernel driver uses these symbols. It is fully open source and
not proprietary. If this is merged, it will break the driver and it will be
incredibly difficult to deal with it while maintaing the ability to respond to
system memory pressure. The kernel just does not provide any other hooks to my
knowledge.