All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Do not call kref utility functions from static inline code
@ 2017-04-18 17:55 Andy Ritger
  2017-04-18 17:55 ` [PATCH 1/2] drm: " Andy Ritger
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andy Ritger @ 2017-04-18 17:55 UTC (permalink / raw)
  To: dri-devel; +Cc: Andy Ritger

I don't expect these two patches to be terribly popular, but let's see.

Nikhil Mahale described the problem here:

    https://lists.freedesktop.org/archives/dri-devel/2017-April/138731.html

In short:

    Commit 10383aea2f44 ("kref: Implement 'struct kref' using refcount_t")
    updated the kref implementation to use refcount_t.  Commit 29dee3c03abc
    ("locking/refcounts: Out-of-line everything") changed the refcount_t
    utility functions from static inline to EXPORT_SYMBOL_GPL functions.

    Through a chain of drm -> kref -> refcount static inline utility
    functions, drm drivers can inadvertently pick up a reference to the
    EXPORT_SYMBOL_GPL refcount_t functions.

To be clear: in the case of NVIDIA's dGPU driver, all the code that
interfaces with drm is in the MIT-licensed nvidia-drm.ko kernel module
(i.e., what Daniel Vetter suggested in response to Nikhil).  Churn in
the drm interfaces exposed to drm drivers is fine from NVIDIA's point
of view, and the burden is NVIDIA's to maintain an out-of-tree drm driver.

Elsewhere in that thread, Lukas Wunner asked why nvidia-drm.ko is licensed
MIT rather than MIT+GPL:

    https://lists.freedesktop.org/archives/dri-devel/2017-April/139033.html

I explained:

    We intentionally chose MODULE_LICENSE("MIT") for nvidia-drm.ko, rather
    than MODULE_LICENSE("Dual MIT/GPL"), to avoid any ambiguity:

    * nvidia-drm.ko depends on nvidia.ko, which is MODULE_LICENSE("NVIDIA").

    * nvidia-drm.ko is the portion of the NVIDIA dGPU driver that interfaces
      with DRM in the kernel.  We wouldn't want nvidia-drm.ko to
      inadvertently function as, or even be perceived as, a path for
      nvidia.ko to indirectly get access to EXPORT_SYMBOL_GPL symbols.

If it is reasonable for there to be MIT drm drivers, then hopefully it
is reasonable for the interface exported by drm to drm drivers to not
require EXPORT_SYMBOL_GPL.

I admit that losing static inline on the functions in these patches is
unfortunate.  Sorry.  At least, I don't expect any of these functions
would be used in particularly perf-critical paths.

I should also note that I attempted to remove all kref helper function
calls from static inline drm functions.  This is more than strictly
necessary for NVIDIA's needs.  But, it seemed good to be consistent.


Andy Ritger (2):
  drm: Do not call kref utility functions from static inline code
  dma-fence: Do not call kref utility functions from static inline code

 drivers/dma-buf/dma-fence.c       | 41 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_atomic.c      | 51 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_framebuffer.c | 12 +++++++++
 drivers/gpu/drm/drm_gem.c         | 35 +++++++++++++++++++++++++++
 include/drm/drm_atomic.h          | 50 +++-----------------------------------
 include/drm/drm_framebuffer.h     | 12 +--------
 include/drm/drm_gem.h             | 36 ++-------------------------
 include/linux/dma-fence.h         | 40 +++---------------------------
 8 files changed, 149 insertions(+), 128 deletions(-)

-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] drm: Do not call kref utility functions from static inline code
  2017-04-18 17:55 [PATCH 0/2] Do not call kref utility functions from static inline code Andy Ritger
@ 2017-04-18 17:55 ` Andy Ritger
  2017-04-18 17:55 ` [PATCH 2/2] dma-fence: " Andy Ritger
  2017-04-18 20:26 ` [PATCH 0/2] " Daniel Vetter
  2 siblings, 0 replies; 5+ messages in thread
From: Andy Ritger @ 2017-04-18 17:55 UTC (permalink / raw)
  To: dri-devel; +Cc: Andy Ritger

Commit 10383aea2f44 ("kref: Implement 'struct kref' using refcount_t")
updated the kref implementation to use refcount_t.  Commit 29dee3c03abc
("locking/refcounts: Out-of-line everything") changed the refcount_t
utility functions from static inline to EXPORT_SYMBOL_GPL functions.

Through a chain of drm -> kref -> refcount static inline utility
functions, drm drivers can inadvertently pick up a reference to the
EXPORT_SYMBOL_GPL refcount_t functions.

refcount_t is an internal implementation detail and not intended as
an interface.  Thus, EXPORT_SYMBOL_GPL seems reasonable for it.

Higher-level drm functions, on the other hand, are intended as an
interface exposed to drm drivers.  So, it arguably seems reasonable that
drm functions do not require EXPORT_SYMBOL_GPL.

Move drm functions from static inline to EXPORT_SYMBOL functions, to
make the interface boundary clear.

Specifically:

Move:
  drm_crtc_commit_get
  drm_crtc_commit_put
  drm_atomic_state_get
  drm_atomic_state_put
from drm_atomic.h to drm_atomic.c.

Move
  drm_framebuffer_read_refcount
from drm_framebuffer.h to drm_framebuffer.c.

Move
  drm_gem_object_reference
  __drm_gem_object_unreference
from drm_gem.h to drm_gem.c.

Signed-off-by: Andy Ritger <aritger@nvidia.com>
---
 drivers/gpu/drm/drm_atomic.c      | 51 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_framebuffer.c | 12 +++++++++
 drivers/gpu/drm/drm_gem.c         | 35 +++++++++++++++++++++++++++
 include/drm/drm_atomic.h          | 50 +++-----------------------------------
 include/drm/drm_framebuffer.h     | 12 +--------
 include/drm/drm_gem.h             | 36 ++-------------------------
 6 files changed, 105 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index a5673107db26..84cc92035a97 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -45,6 +45,31 @@ void __drm_crtc_commit_free(struct kref *kref)
 EXPORT_SYMBOL(__drm_crtc_commit_free);
 
 /**
+ * drm_crtc_commit_get - acquire a reference to the CRTC commit
+ * @commit: CRTC commit
+ *
+ * Increases the reference of @commit.
+ */
+void drm_crtc_commit_get(struct drm_crtc_commit *commit)
+{
+	kref_get(&commit->ref);
+}
+EXPORT_SYMBOL(drm_crtc_commit_get);
+
+/**
+ * drm_crtc_commit_put - release a reference to the CRTC commmit
+ * @commit: CRTC commit
+ *
+ * This releases a reference to @commit which is freed after removing the
+ * final reference. No locking required and callable from any context.
+ */
+void drm_crtc_commit_put(struct drm_crtc_commit *commit)
+{
+	kref_put(&commit->ref, __drm_crtc_commit_free);
+}
+EXPORT_SYMBOL(drm_crtc_commit_put);
+
+/**
  * drm_atomic_state_default_release -
  * release memory initialized by drm_atomic_state_init
  * @state: atomic state
@@ -239,6 +264,32 @@ void __drm_atomic_state_free(struct kref *ref)
 EXPORT_SYMBOL(__drm_atomic_state_free);
 
 /**
+ * drm_atomic_state_get - acquire a reference to the atomic state
+ * @state: The atomic state
+ *
+ * Returns a new reference to the @state
+ */
+struct drm_atomic_state *drm_atomic_state_get(struct drm_atomic_state *state)
+{
+	kref_get(&state->ref);
+	return state;
+}
+EXPORT_SYMBOL(drm_atomic_state_get);
+
+/**
+ * drm_atomic_state_put - release a reference to the atomic state
+ * @state: The atomic state
+ *
+ * This releases a reference to @state which is freed after removing the
+ * final reference. No locking required and callable from any context.
+ */
+void drm_atomic_state_put(struct drm_atomic_state *state)
+{
+	kref_put(&state->ref, __drm_atomic_state_free);
+}
+EXPORT_SYMBOL(drm_atomic_state_put);
+
+/**
  * drm_atomic_get_crtc_state - get crtc state
  * @state: global atomic state object
  * @crtc: crtc to get state object for
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 28a0108a1ab8..6f24ba5e3866 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -837,3 +837,15 @@ int drm_framebuffer_plane_height(int height,
 	return height / fb->format->vsub;
 }
 EXPORT_SYMBOL(drm_framebuffer_plane_height);
+
+/**
+ * drm_framebuffer_read_refcount - read the framebuffer reference count.
+ * @fb: framebuffer
+ *
+ * This functions returns the framebuffer's reference count.
+ */
+uint32_t drm_framebuffer_read_refcount(struct drm_framebuffer *fb)
+{
+	return kref_read(&fb->base.refcount);
+}
+EXPORT_SYMBOL(drm_framebuffer_read_refcount);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index bc93de308673..d189b5720bcb 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1004,3 +1004,38 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	return ret;
 }
 EXPORT_SYMBOL(drm_gem_mmap);
+
+/**
+ * drm_gem_object_reference - acquire a GEM BO reference
+ * @obj: GEM buffer object
+ *
+ * This acquires additional reference to @obj. It is illegal to call this
+ * without already holding a reference. No locks required.
+ */
+void drm_gem_object_reference(struct drm_gem_object *obj)
+{
+	kref_get(&obj->refcount);
+}
+EXPORT_SYMBOL(drm_gem_object_reference);
+
+/**
+ * __drm_gem_object_unreference - raw function to release a GEM BO reference
+ * @obj: GEM buffer object
+ *
+ * This function is meant to be used by drivers which are not encumbered with
+ * &drm_device.struct_mutex legacy locking and which are using the
+ * gem_free_object_unlocked callback. It avoids all the locking checks and
+ * locking overhead of drm_gem_object_unreference() and
+ * drm_gem_object_unreference_unlocked().
+ *
+ * Drivers should never call this directly in their code. Instead they should
+ * wrap it up into a ``driver_gem_object_unreference(struct driver_gem_object
+ * *obj)`` wrapper function, and use that. Shared code should never call this,
+ * to avoid breaking drivers by accident which still depend upon
+ * &drm_device.struct_mutex locking.
+ */
+void __drm_gem_object_unreference(struct drm_gem_object *obj)
+{
+	kref_put(&obj->refcount, drm_gem_object_free);
+}
+EXPORT_SYMBOL(__drm_gem_object_unreference);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 052ab161b239..60f93d084fe4 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -192,59 +192,17 @@ struct drm_atomic_state {
 
 void __drm_crtc_commit_free(struct kref *kref);
 
-/**
- * drm_crtc_commit_get - acquire a reference to the CRTC commit
- * @commit: CRTC commit
- *
- * Increases the reference of @commit.
- */
-static inline void drm_crtc_commit_get(struct drm_crtc_commit *commit)
-{
-	kref_get(&commit->ref);
-}
-
-/**
- * drm_crtc_commit_put - release a reference to the CRTC commmit
- * @commit: CRTC commit
- *
- * This releases a reference to @commit which is freed after removing the
- * final reference. No locking required and callable from any context.
- */
-static inline void drm_crtc_commit_put(struct drm_crtc_commit *commit)
-{
-	kref_put(&commit->ref, __drm_crtc_commit_free);
-}
+void drm_crtc_commit_get(struct drm_crtc_commit *commit);
+void drm_crtc_commit_put(struct drm_crtc_commit *commit);
 
 struct drm_atomic_state * __must_check
 drm_atomic_state_alloc(struct drm_device *dev);
 void drm_atomic_state_clear(struct drm_atomic_state *state);
 
-/**
- * drm_atomic_state_get - acquire a reference to the atomic state
- * @state: The atomic state
- *
- * Returns a new reference to the @state
- */
-static inline struct drm_atomic_state *
-drm_atomic_state_get(struct drm_atomic_state *state)
-{
-	kref_get(&state->ref);
-	return state;
-}
-
 void __drm_atomic_state_free(struct kref *ref);
 
-/**
- * drm_atomic_state_put - release a reference to the atomic state
- * @state: The atomic state
- *
- * This releases a reference to @state which is freed after removing the
- * final reference. No locking required and callable from any context.
- */
-static inline void drm_atomic_state_put(struct drm_atomic_state *state)
-{
-	kref_put(&state->ref, __drm_atomic_state_free);
-}
+struct drm_atomic_state *drm_atomic_state_get(struct drm_atomic_state *state);
+void drm_atomic_state_put(struct drm_atomic_state *state);
 
 int  __must_check
 drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state);
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index dd1e3e99dcff..b6c7f6aec9df 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -202,6 +202,7 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
 void drm_framebuffer_remove(struct drm_framebuffer *fb);
 void drm_framebuffer_cleanup(struct drm_framebuffer *fb);
 void drm_framebuffer_unregister_private(struct drm_framebuffer *fb);
+uint32_t drm_framebuffer_read_refcount(struct drm_framebuffer *fb);
 
 /**
  * drm_framebuffer_reference - incr the fb refcnt
@@ -226,17 +227,6 @@ static inline void drm_framebuffer_unreference(struct drm_framebuffer *fb)
 }
 
 /**
- * drm_framebuffer_read_refcount - read the framebuffer reference count.
- * @fb: framebuffer
- *
- * This functions returns the framebuffer's reference count.
- */
-static inline uint32_t drm_framebuffer_read_refcount(struct drm_framebuffer *fb)
-{
-	return kref_read(&fb->base.refcount);
-}
-
-/**
  * drm_framebuffer_assign - store a reference to the fb
  * @p: location to store framebuffer
  * @fb: new framebuffer (maybe NULL)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 449a41b56ffc..e66bd3110044 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -186,40 +186,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 		     struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
-/**
- * drm_gem_object_reference - acquire a GEM BO reference
- * @obj: GEM buffer object
- *
- * This acquires additional reference to @obj. It is illegal to call this
- * without already holding a reference. No locks required.
- */
-static inline void
-drm_gem_object_reference(struct drm_gem_object *obj)
-{
-	kref_get(&obj->refcount);
-}
-
-/**
- * __drm_gem_object_unreference - raw function to release a GEM BO reference
- * @obj: GEM buffer object
- *
- * This function is meant to be used by drivers which are not encumbered with
- * &drm_device.struct_mutex legacy locking and which are using the
- * gem_free_object_unlocked callback. It avoids all the locking checks and
- * locking overhead of drm_gem_object_unreference() and
- * drm_gem_object_unreference_unlocked().
- *
- * Drivers should never call this directly in their code. Instead they should
- * wrap it up into a ``driver_gem_object_unreference(struct driver_gem_object
- * *obj)`` wrapper function, and use that. Shared code should never call this, to
- * avoid breaking drivers by accident which still depend upon
- * &drm_device.struct_mutex locking.
- */
-static inline void
-__drm_gem_object_unreference(struct drm_gem_object *obj)
-{
-	kref_put(&obj->refcount, drm_gem_object_free);
-}
+void drm_gem_object_reference(struct drm_gem_object *obj);
+void __drm_gem_object_unreference(struct drm_gem_object *obj);
 
 void drm_gem_object_unreference_unlocked(struct drm_gem_object *obj);
 void drm_gem_object_unreference(struct drm_gem_object *obj);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] dma-fence: Do not call kref utility functions from static inline code
  2017-04-18 17:55 [PATCH 0/2] Do not call kref utility functions from static inline code Andy Ritger
  2017-04-18 17:55 ` [PATCH 1/2] drm: " Andy Ritger
@ 2017-04-18 17:55 ` Andy Ritger
  2017-04-18 20:26 ` [PATCH 0/2] " Daniel Vetter
  2 siblings, 0 replies; 5+ messages in thread
From: Andy Ritger @ 2017-04-18 17:55 UTC (permalink / raw)
  To: dri-devel; +Cc: Andy Ritger

Commit 10383aea2f44 ("kref: Implement 'struct kref' using refcount_t")
updated the kref implementation to use refcount_t.  Commit 29dee3c03abc
("locking/refcounts: Out-of-line everything") changed the refcount_t
utility functions from static inline to EXPORT_SYMBOL_GPL functions.

Through a chain of dma-fence -> kref -> refcount static inline utility
functions, drm drivers can inadvertently pick up a reference to the
EXPORT_SYMBOL_GPL refcount_t functions.

refcount_t is an internal implementation detail and not intended as
an interface.  Thus, EXPORT_SYMBOL_GPL seems reasonable for it.

Higher-level dma-fence functions, on the other hand, are intended as an
interface exposed to drm drivers.  So, it arguably seems reasonable that
dma-fence functions do not require EXPORT_SYMBOL_GPL.

Move dma_fence_{put,get,get_rcu} from static inline in dma-fence.h to
EXPORT_SYMBOL functions in dma-fence, to make the interface boundary clear.

Signed-off-by: Andy Ritger <aritger@nvidia.com>
---
 drivers/dma-buf/dma-fence.c | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/dma-fence.h   | 40 +++-------------------------------------
 2 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index d195d617076d..863bf55955ef 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -573,3 +573,44 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 	trace_dma_fence_init(fence);
 }
 EXPORT_SYMBOL(dma_fence_init);
+
+/**
+ * dma_fence_put - decreases refcount of the fence
+ * @fence:      [in]    fence to reduce refcount of
+ */
+void dma_fence_put(struct dma_fence *fence)
+{
+	if (fence)
+		kref_put(&fence->refcount, dma_fence_release);
+}
+EXPORT_SYMBOL(dma_fence_put);
+
+/**
+ * dma_fence_get - increases refcount of the fence
+ * @fence:      [in]    fence to increase refcount of
+ *
+ * Returns the same fence, with refcount increased by 1.
+ */
+struct dma_fence *dma_fence_get(struct dma_fence *fence)
+{
+	if (fence)
+		kref_get(&fence->refcount);
+	return fence;
+}
+EXPORT_SYMBOL(dma_fence_get);
+
+/**
+ * dma_fence_get_rcu - get a fence from a reservation_object_list with
+ *                     rcu read lock
+ * @fence:      [in]    fence to increase refcount of
+ *
+ * Function returns NULL if no refcount could be obtained, or the fence.
+ */
+struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
+{
+	if (kref_get_unless_zero(&fence->refcount))
+		return fence;
+	else
+		return NULL;
+}
+EXPORT_SYMBOL(dma_fence_get_rcu);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 6048fa404e57..0e8d714a5bc7 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -185,43 +185,9 @@ void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 void dma_fence_release(struct kref *kref);
 void dma_fence_free(struct dma_fence *fence);
 
-/**
- * dma_fence_put - decreases refcount of the fence
- * @fence:	[in]	fence to reduce refcount of
- */
-static inline void dma_fence_put(struct dma_fence *fence)
-{
-	if (fence)
-		kref_put(&fence->refcount, dma_fence_release);
-}
-
-/**
- * dma_fence_get - increases refcount of the fence
- * @fence:	[in]	fence to increase refcount of
- *
- * Returns the same fence, with refcount increased by 1.
- */
-static inline struct dma_fence *dma_fence_get(struct dma_fence *fence)
-{
-	if (fence)
-		kref_get(&fence->refcount);
-	return fence;
-}
-
-/**
- * dma_fence_get_rcu - get a fence from a reservation_object_list with
- *                     rcu read lock
- * @fence:	[in]	fence to increase refcount of
- *
- * Function returns NULL if no refcount could be obtained, or the fence.
- */
-static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
-{
-	if (kref_get_unless_zero(&fence->refcount))
-		return fence;
-	else
-		return NULL;
-}
+void dma_fence_put(struct dma_fence *fence);
+struct dma_fence *dma_fence_get(struct dma_fence *fence);
+struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence);
 
 /**
  * dma_fence_get_rcu_safe  - acquire a reference to an RCU tracked fence
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Do not call kref utility functions from static inline code
  2017-04-18 17:55 [PATCH 0/2] Do not call kref utility functions from static inline code Andy Ritger
  2017-04-18 17:55 ` [PATCH 1/2] drm: " Andy Ritger
  2017-04-18 17:55 ` [PATCH 2/2] dma-fence: " Andy Ritger
@ 2017-04-18 20:26 ` Daniel Vetter
  2017-04-19  5:37   ` Andy Ritger
  2 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2017-04-18 20:26 UTC (permalink / raw)
  To: Andy Ritger; +Cc: dri-devel

On Tue, Apr 18, 2017 at 7:55 PM, Andy Ritger <aritger@nvidia.com> wrote:
> I explained:
>
>     We intentionally chose MODULE_LICENSE("MIT") for nvidia-drm.ko, rather
>     than MODULE_LICENSE("Dual MIT/GPL"), to avoid any ambiguity:
>
>     * nvidia-drm.ko depends on nvidia.ko, which is MODULE_LICENSE("NVIDIA").
>
>     * nvidia-drm.ko is the portion of the NVIDIA dGPU driver that interfaces
>       with DRM in the kernel.  We wouldn't want nvidia-drm.ko to
>       inadvertently function as, or even be perceived as, a path for
>       nvidia.ko to indirectly get access to EXPORT_SYMBOL_GPL symbols.

So I'm a layman and all that, but I don't get why this is relevant at
all. Speaking for myself, not my employer, not legal advice, yadayada,
but I see two options:

- the blob has become at least partially a derived work of the linux
kernel work. You're fucked, whether you use gpl only symbols or not,
because if there ever was some meaning to labelling stuff gpl-only it
has long evaporated imo and become a pure political thing.

- it is still a clearly independent work. There's no problem, because
only the nvidia-drm impendence mismatch layer is a derived work (no on
argues that I guess), and that along could easily ship as a part of
the kernel with the overall GPL license. Of course upstream wont do
that because it'd be pointless, but I don't see a problem.

So either labelling it as dual MIT/GPL isn't really a problem, or
labelling it as MIT-only isn't going to save your (legally speaking at
least).

Doing simple wrappers around gpl-only symbols otoh like you do here
isn't any different from shipping it as part of nvidia-drm, at least I
don't see how this factually changes anything. So I don't see any
benefit for you folks. And if you are already infringing it's a pretty
obvious circumvention trick, so definitely not something I can nor
will get involved with or even look like I'm approving. Imo
drm_prime.c was already borderline, but could be justified with code
sharing on technical grounds post-factum (or post-merging).

Overall I still don't get why you do this, and how this can help
exactly. You seem to already have the architecture that I think is the
only option which might be possible to be shipped without angering
someone somewhere too much to give you (legal) heat. And it's also the
only architecture that make sense technically.

/me confused
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Do not call kref utility functions from static inline code
  2017-04-18 20:26 ` [PATCH 0/2] " Daniel Vetter
@ 2017-04-19  5:37   ` Andy Ritger
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Ritger @ 2017-04-19  5:37 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, Andy Ritger

On Tue, Apr 18, 2017 at 10:26:39PM +0200, Daniel Vetter wrote:
> On Tue, Apr 18, 2017 at 7:55 PM, Andy Ritger <aritger@nvidia.com> wrote:
> > I explained:
> >
> >     We intentionally chose MODULE_LICENSE("MIT") for nvidia-drm.ko, rather
> >     than MODULE_LICENSE("Dual MIT/GPL"), to avoid any ambiguity:
> >
> >     * nvidia-drm.ko depends on nvidia.ko, which is MODULE_LICENSE("NVIDIA").
> >
> >     * nvidia-drm.ko is the portion of the NVIDIA dGPU driver that interfaces
> >       with DRM in the kernel.  We wouldn't want nvidia-drm.ko to
> >       inadvertently function as, or even be perceived as, a path for
> >       nvidia.ko to indirectly get access to EXPORT_SYMBOL_GPL symbols.
> 
> So I'm a layman and all that, but I don't get why this is relevant at
> all. Speaking for myself, not my employer, not legal advice, yadayada,
> but I see two options:
> 
> - the blob has become at least partially a derived work of the linux
> kernel work. You're fucked, whether you use gpl only symbols or not,
> because if there ever was some meaning to labelling stuff gpl-only it
> has long evaporated imo and become a pure political thing.
> 
> - it is still a clearly independent work. There's no problem, because
> only the nvidia-drm impendence mismatch layer is a derived work (no on
> argues that I guess), and that along could easily ship as a part of
> the kernel with the overall GPL license. Of course upstream wont do
> that because it'd be pointless, but I don't see a problem.
> 
> So either labelling it as dual MIT/GPL isn't really a problem, or
> labelling it as MIT-only isn't going to save your (legally speaking at
> least).
> 
> Doing simple wrappers around gpl-only symbols otoh like you do here
> isn't any different from shipping it as part of nvidia-drm, at least I
> don't see how this factually changes anything.

From my perspective, the distinction is:

* How drm functions are implemented is an implementation detail of drm.
  The implementations of these functions use kref/refcount_t today because
  they are well-proven primitives in the kernel.  But, they could just
  as easily have an entirely different implementation, and drm drivers
  don't need to care.

* In contrast, if nvidia-drm directly accesses kref/refcount_t, we were
  worried it would be perceived by some as an attempt to side-step the
  intent of refcount_t being EXPORT_SYMBOL_GPL in the first place.
  My goal was just for nvidia-drm to plug into the drm subsystem, not to
  use any facilities that were deemed low-level implementation details
  of the kernel.

Thanks,
- Andy

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-04-19  5:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 17:55 [PATCH 0/2] Do not call kref utility functions from static inline code Andy Ritger
2017-04-18 17:55 ` [PATCH 1/2] drm: " Andy Ritger
2017-04-18 17:55 ` [PATCH 2/2] dma-fence: " Andy Ritger
2017-04-18 20:26 ` [PATCH 0/2] " Daniel Vetter
2017-04-19  5:37   ` Andy Ritger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.