Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1635336 > unrolled thread

Re: [GIT PULL] locking fixes

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [GIT PULL] locking fixes Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-04 01:30 +0200
    Re: [GIT PULL] locking fixes Peter Zijlstra <peterz@infradead.org> - 2017-05-04 07:50 +0200
      Re: [GIT PULL] locking fixes Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-05-05 00:50 +0200
        refcount: change EXPORT_SYMBOL markings Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-05-05 01:00 +0200

#1635336 — Re: [GIT PULL] locking fixes

This is from last merge window, and the reason I react now is that
nobody noticed or cared until we had a release..

On Mon, Feb 27, 2017 at 11:57 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> Peter Zijlstra (1):
>       locking/refcounts: Out-of-line everything

This one is all good generally, but it has one really stupid side
effect: it makes refcounting GPL-only.

That's just silly. These are functions that atomically add and
subtract one. The only thing that making them GPL-only can possibly do
is to make people hack around it, and lose the overflow handling
debugging in the process.

It also breaks any kref uses. Which is what drivers etc are supposed to use.

So that "move from inline to out-of-line" had a big subtle semantic
change that was probably not intentional, and certainly not
documented.

                  Linus

[toc] | [next] | [standalone]

#1635422

On Wed, May 03, 2017 at 04:21:01PM -0700, Linus Torvalds wrote:
> This is from last merge window, and the reason I react now is that
> nobody noticed or cared until we had a release..
> 
> On Mon, Feb 27, 2017 at 11:57 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > Peter Zijlstra (1):
> >       locking/refcounts: Out-of-line everything
> 
> This one is all good generally, but it has one really stupid side
> effect: it makes refcounting GPL-only.
> 
> That's just silly. These are functions that atomically add and
> subtract one. The only thing that making them GPL-only can possibly do
> is to make people hack around it, and lose the overflow handling
> debugging in the process.

These people are out-of-tree dubious licensed modules, right? I really
_really_ don't care about those.

> It also breaks any kref uses. Which is what drivers etc are supposed to use.

Greg KH had this to say:

"As all of the previous kref functions were in a GPL-only header file,
and included directly that way, they were already GPL-only symbols, so
there really was no change here except now the linker checks them.  If
you have questions about using inline GPL-only functions from a .h file,
in a non-GPL codebase, please consult your corporate lawyer to get
clarification."

 https://lkml.kernel.org/r/20170308094810.GB30552@kroah.com

> So that "move from inline to out-of-line" had a big subtle semantic
> change that was probably not intentional, and certainly not
> documented.

I'll take the not documented bit.

[toc] | [prev] | [next] | [standalone]

#1636069

On Wed, May 03, 2017 at 11:10:07PM -0700, Linus Torvalds wrote:
> 
> 
> On May 3, 2017 22:40, "Peter Zijlstra" <peterz@infradead.org> wrote:
> 
> 
>     These people are out-of-tree dubious licensed modules, right? I really
>     _really_ don't care about those.
> 
> 
> Mainly Nvidia, I think.

nvidia said it was for "new code" they were still developing, and that
the license issue was on their side, and they fixed it up.  They also
agreed that the change was ok from their point of view.

> But the point is, you broke people's working setups.
> 
> We don't do that.

We have never guaranteed kernel api stability, but yes, this is
different from that.

Moving these from a .h to .c caused the change, I asked for this as the
original symbols were obviously GPL-only being in a .h file.  However I
understand your point, we don't want to have people any grumpier at us
than normal :)

> Perhaps equally importantly, you did it by marking *trivial* functions that are
> definitely meant for drivers as gpl-only. Which only demeans the whole concept
> that we consider the gpl-only thing to be an "internal kernel function".

These are really now low-level kernel functions, and not trivial ones
given the long long email threads on how to get them all working
properly.  This was obviously something that took a lot of time to do.

> So the change actually makes our explicitly stated arguments for why certain
> functions are special less valid, and replaced it with a stupid grandstanding
> and legally dubious "linking means it's a derived work" argument.

There's no "linking" argument here, I didn't make that.

But again, I understand the point, changing api "markings" like this
"mid-stream" isn't the nicest thing to do.  And hey, I'm trying to be
"meaner" as I think someone once told me to be that way, so I
recommended that Peter make this change :)

I'll send a patch to change these back now...

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1636071 — refcount: change EXPORT_SYMBOL markings

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Now that kref is using the refcount apis, the _GPL markings are getting
exported to places that it previously wasn't.  Now kref.h is GPLv2
licensed, so any non-GPL code using it better be talking to some
lawyers, but changing api markings isn't considered "nice", so let's fix
this up.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/lib/refcount.c b/lib/refcount.c
index f42124ccf295..9f906783987e 100644
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -76,7 +76,7 @@ bool refcount_add_not_zero(unsigned int i, refcount_t *r)
 
 	return true;
 }
-EXPORT_SYMBOL_GPL(refcount_add_not_zero);
+EXPORT_SYMBOL(refcount_add_not_zero);
 
 /**
  * refcount_add - add a value to a refcount
@@ -98,7 +98,7 @@ void refcount_add(unsigned int i, refcount_t *r)
 {
 	WARN_ONCE(!refcount_add_not_zero(i, r), "refcount_t: addition on 0; use-after-free.\n");
 }
-EXPORT_SYMBOL_GPL(refcount_add);
+EXPORT_SYMBOL(refcount_add);
 
 /**
  * refcount_inc_not_zero - increment a refcount unless it is 0
@@ -131,7 +131,7 @@ bool refcount_inc_not_zero(refcount_t *r)
 
 	return true;
 }
-EXPORT_SYMBOL_GPL(refcount_inc_not_zero);
+EXPORT_SYMBOL(refcount_inc_not_zero);
 
 /**
  * refcount_inc - increment a refcount
@@ -149,7 +149,7 @@ void refcount_inc(refcount_t *r)
 {
 	WARN_ONCE(!refcount_inc_not_zero(r), "refcount_t: increment on 0; use-after-free.\n");
 }
-EXPORT_SYMBOL_GPL(refcount_inc);
+EXPORT_SYMBOL(refcount_inc);
 
 /**
  * refcount_sub_and_test - subtract from a refcount and test if it is 0
@@ -189,7 +189,7 @@ bool refcount_sub_and_test(unsigned int i, refcount_t *r)
 
 	return !new;
 }
-EXPORT_SYMBOL_GPL(refcount_sub_and_test);
+EXPORT_SYMBOL(refcount_sub_and_test);
 
 /**
  * refcount_dec_and_test - decrement a refcount and test if it is 0
@@ -208,7 +208,7 @@ bool refcount_dec_and_test(refcount_t *r)
 {
 	return refcount_sub_and_test(1, r);
 }
-EXPORT_SYMBOL_GPL(refcount_dec_and_test);
+EXPORT_SYMBOL(refcount_dec_and_test);
 
 /**
  * refcount_dec - decrement a refcount
@@ -224,7 +224,7 @@ void refcount_dec(refcount_t *r)
 {
 	WARN_ONCE(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n");
 }
-EXPORT_SYMBOL_GPL(refcount_dec);
+EXPORT_SYMBOL(refcount_dec);
 
 /**
  * refcount_dec_if_one - decrement a refcount if it is 1
@@ -248,7 +248,7 @@ bool refcount_dec_if_one(refcount_t *r)
 
 	return atomic_try_cmpxchg_release(&r->refs, &val, 0);
 }
-EXPORT_SYMBOL_GPL(refcount_dec_if_one);
+EXPORT_SYMBOL(refcount_dec_if_one);
 
 /**
  * refcount_dec_not_one - decrement a refcount if it is not 1
@@ -282,7 +282,7 @@ bool refcount_dec_not_one(refcount_t *r)
 
 	return true;
 }
-EXPORT_SYMBOL_GPL(refcount_dec_not_one);
+EXPORT_SYMBOL(refcount_dec_not_one);
 
 /**
  * refcount_dec_and_mutex_lock - return holding mutex if able to decrement
@@ -313,7 +313,7 @@ bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock)
 
 	return true;
 }
-EXPORT_SYMBOL_GPL(refcount_dec_and_mutex_lock);
+EXPORT_SYMBOL(refcount_dec_and_mutex_lock);
 
 /**
  * refcount_dec_and_lock - return holding spinlock if able to decrement
@@ -344,5 +344,5 @@ bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock)
 
 	return true;
 }
-EXPORT_SYMBOL_GPL(refcount_dec_and_lock);
+EXPORT_SYMBOL(refcount_dec_and_lock);
 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web