[PATCH 0/4] dma-buf/reservation doc updates

[PATCH 0/4] dma-buf/reservation doc updates

[Rob Clark]

Some dma-buf headerdoc fixes (turns out dma-buf.h wasn't getting pulled
into doc build), add reservation headerdoc, and fixup a bit the section
in device-drivers.tmpl.

Rob Clark (4):
  reservation: sprinkle some WARN_ON()s
  dma-buf: headerdoc fixes
  reservation: add headerdoc comments
  doc: update/fixup dma-buf related DocBook

 Documentation/DocBook/device-drivers.tmpl | 36 ++++++++++++--
 drivers/dma-buf/reservation.c             | 78 +++++++++++++++++++++++++++++--
 include/linux/dma-buf.h                   | 13 ++++--
 include/linux/reservation.h               | 53 +++++++++++++++++++++
 4 files changed, 169 insertions(+), 11 deletions(-)

-- 
2.5.5

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

[PATCH 1/4] reservation: sprinkle some WARN_ON()s

[Rob Clark]

A bit overkill since, for example, the rcu_dereference_protected() in
reservation_object_get_list() will WARN.  But this is much less subtle
for folks reading the code.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/dma-buf/reservation.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index c0bd572..0de3ea6 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
 	struct reservation_object_list *fobj, *old;
 	u32 max;
 
+	WARN_ON(!ww_mutex_is_locked(&obj->lock));
+
 	old = reservation_object_get_list(obj);
 
 	if (old && old->shared_max) {
@@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
 {
 	struct reservation_object_list *old, *fobj = obj->staged;
 
+	WARN_ON(!ww_mutex_is_locked(&obj->lock));
+
 	old = reservation_object_get_list(obj);
 	obj->staged = NULL;
 
@@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
 	struct reservation_object_list *old;
 	u32 i = 0;
 
+	WARN_ON(!ww_mutex_is_locked(&obj->lock));
+
 	old = reservation_object_get_list(obj);
 	if (old)
 		i = old->shared_count;
-- 
2.5.5

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

Re: [PATCH 1/4] reservation: sprinkle some WARN_ON()s

[Lucas Stach]

Am Donnerstag, den 31.03.2016, 16:26 -0400 schrieb Rob Clark:
> A bit overkill since, for example, the rcu_dereference_protected() in
> reservation_object_get_list() will WARN.  But this is much less subtle
> for folks reading the code.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/dma-buf/reservation.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index c0bd572..0de3ea6 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
>  	struct reservation_object_list *fobj, *old;
>  	u32 max;
>  
> +	WARN_ON(!ww_mutex_is_locked(&obj->lock));
> +
I don't really like those WARN_ONs to enforce the expected locking.

IMHO lockdep_assert_held is the better way, as it's as obvious as the
WARN_ONs to someone reading the code, blows up in the same way if you
are using a lockdep enabled build (which should be default when touching
locking code) and avoids the runtime overhead for production kernels.

It also checks that it's really the expected execution strand holding
the lock and not some other thread on another CPU.

I don't know if lockdep_assert_held works with ww_mutex currently, if
not we should definitely extend it to do so.

Regards,
Lucas

>  	old = reservation_object_get_list(obj);
>  
>  	if (old && old->shared_max) {
> @@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
>  {
>  	struct reservation_object_list *old, *fobj = obj->staged;
>  
> +	WARN_ON(!ww_mutex_is_locked(&obj->lock));
> +
>  	old = reservation_object_get_list(obj);
>  	obj->staged = NULL;
>  
> @@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
>  	struct reservation_object_list *old;
>  	u32 i = 0;
>  
> +	WARN_ON(!ww_mutex_is_locked(&obj->lock));
> +
>  	old = reservation_object_get_list(obj);
>  	if (old)
>  		i = old->shared_count;


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

Re: [PATCH 1/4] reservation: sprinkle some WARN_ON()s

[Rob Clark]

On Fri, Apr 1, 2016 at 4:48 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Donnerstag, den 31.03.2016, 16:26 -0400 schrieb Rob Clark:
>> A bit overkill since, for example, the rcu_dereference_protected() in
>> reservation_object_get_list() will WARN.  But this is much less subtle
>> for folks reading the code.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  drivers/dma-buf/reservation.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
>> index c0bd572..0de3ea6 100644
>> --- a/drivers/dma-buf/reservation.c
>> +++ b/drivers/dma-buf/reservation.c
>> @@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
>>       struct reservation_object_list *fobj, *old;
>>       u32 max;
>>
>> +     WARN_ON(!ww_mutex_is_locked(&obj->lock));
>> +
> I don't really like those WARN_ONs to enforce the expected locking.
>
> IMHO lockdep_assert_held is the better way, as it's as obvious as the
> WARN_ONs to someone reading the code, blows up in the same way if you
> are using a lockdep enabled build (which should be default when touching
> locking code) and avoids the runtime overhead for production kernels.
>
> It also checks that it's really the expected execution strand holding
> the lock and not some other thread on another CPU.
>
> I don't know if lockdep_assert_held works with ww_mutex currently, if
> not we should definitely extend it to do so.

I guess it should, since there is reservation_object_held()..  I
suppose I should use this.

I suppose all the WARN_ON(!drm_modeset_is_locked()) should someday get
the same treatment..

BR,
-R

> Regards,
> Lucas
>
>>       old = reservation_object_get_list(obj);
>>
>>       if (old && old->shared_max) {
>> @@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
>>  {
>>       struct reservation_object_list *old, *fobj = obj->staged;
>>
>> +     WARN_ON(!ww_mutex_is_locked(&obj->lock));
>> +
>>       old = reservation_object_get_list(obj);
>>       obj->staged = NULL;
>>
>> @@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
>>       struct reservation_object_list *old;
>>       u32 i = 0;
>>
>> +     WARN_ON(!ww_mutex_is_locked(&obj->lock));
>> +
>>       old = reservation_object_get_list(obj);
>>       if (old)
>>               i = old->shared_count;
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] reservation: sprinkle some WARN_ON()s

[Christian König]

Am 31.03.2016 um 22:26 schrieb Rob Clark:
> A bit overkill since, for example, the rcu_dereference_protected() in
> reservation_object_get_list() will WARN.  But this is much less subtle
> for folks reading the code.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com> for this one.

One item to purge from my todo list, thanks!

Christian.

> ---
>   drivers/dma-buf/reservation.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index c0bd572..0de3ea6 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
>   	struct reservation_object_list *fobj, *old;
>   	u32 max;
>   
> +	WARN_ON(!ww_mutex_is_locked(&obj->lock));
> +
>   	old = reservation_object_get_list(obj);
>   
>   	if (old && old->shared_max) {
> @@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
>   {
>   	struct reservation_object_list *old, *fobj = obj->staged;
>   
> +	WARN_ON(!ww_mutex_is_locked(&obj->lock));
> +
>   	old = reservation_object_get_list(obj);
>   	obj->staged = NULL;
>   
> @@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
>   	struct reservation_object_list *old;
>   	u32 i = 0;
>   
> +	WARN_ON(!ww_mutex_is_locked(&obj->lock));
> +
>   	old = reservation_object_get_list(obj);
>   	if (old)
>   		i = old->shared_count;

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

[PATCH 2/4] dma-buf: headerdoc fixes

[Rob Clark]

Apparently nobody noticed that dma-buf.h wasn't actually pulled into
docbook build.  And as a result the headerdoc comments bitrot a bit.
Add missing params/fields.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/linux/dma-buf.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index f98bd70..6befeed 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -114,19 +114,24 @@ struct dma_buf_ops {
  * @file: file pointer used for sharing buffers across, and for refcounting.
  * @attachments: list of dma_buf_attachment that denotes all devices attached.
  * @ops: dma_buf_ops associated with this buffer object.
+ * @lock: used internally to serialize list manipulation, attach/detach and vmap/unmap
+ * @vmapping_counter: used internally to refcnt the vmaps
+ * @vmap_ptr: the current vmap ptr if vmapping_counter > 0
  * @exp_name: name of the exporter; useful for debugging.
  * @owner: pointer to exporter module; used for refcounting when exporter is a
  *         kernel module.
  * @list_node: node for dma_buf accounting and debugging.
  * @priv: exporter specific private data for this buffer object.
  * @resv: reservation object linked to this dma-buf
+ * @poll: for userspace poll support
+ * @cb_excl: for userspace poll support
+ * @cb_shared: for userspace poll support
  */
 struct dma_buf {
 	size_t size;
 	struct file *file;
 	struct list_head attachments;
 	const struct dma_buf_ops *ops;
-	/* mutex to serialize list manipulation, attach/detach and vmap/unmap */
 	struct mutex lock;
 	unsigned vmapping_counter;
 	void *vmap_ptr;
@@ -190,9 +195,11 @@ struct dma_buf_export_info {
 
 /**
  * helper macro for exporters; zeros and fills in most common values
+ *
+ * @name: export-info name
  */
-#define DEFINE_DMA_BUF_EXPORT_INFO(a)	\
-	struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME, \
+#define DEFINE_DMA_BUF_EXPORT_INFO(name)	\
+	struct dma_buf_export_info name = { .exp_name = KBUILD_MODNAME, \
 					 .owner = THIS_MODULE }
 
 /**
-- 
2.5.5

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

[PATCH 3/4] reservation: add headerdoc comments

[Rob Clark]

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/dma-buf/reservation.c | 72 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/reservation.h   | 53 +++++++++++++++++++++++++++++++
 2 files changed, 121 insertions(+), 4 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 0de3ea6..0d18952 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -35,6 +35,17 @@
 #include <linux/reservation.h>
 #include <linux/export.h>
 
+/**
+ * DOC: Reservation Object Overview
+ *
+ * The reservation object provides a mechanism to manage shared and
+ * exclusive fences associated with a buffer.  A reservation object
+ * can have attached one exclusive fence (normally associated with
+ * write operations) or N shared fences (read operations).  The RCU
+ * mechanism is used to protect read access to fences from locked
+ * write-side updates.
+ */
+
 DEFINE_WW_CLASS(reservation_ww_class);
 EXPORT_SYMBOL(reservation_ww_class);
 
@@ -43,9 +54,17 @@ EXPORT_SYMBOL(reservation_seqcount_class);
 
 const char reservation_seqcount_string[] = "reservation_seqcount";
 EXPORT_SYMBOL(reservation_seqcount_string);
-/*
- * Reserve space to add a shared fence to a reservation_object,
- * must be called with obj->lock held.
+
+/**
+ * reservation_object_reserve_shared - Reserve space to add a shared
+ * fence to a reservation_object.
+ * @obj: reservation object
+ *
+ * Should be called before reservation_object_add_shared_fence().  Must
+ * be called with obj->lock held.
+ *
+ * RETURNS
+ * Zero for success, or -errno
  */
 int reservation_object_reserve_shared(struct reservation_object *obj)
 {
@@ -182,7 +201,11 @@ done:
 		fence_put(old_fence);
 }
 
-/*
+/**
+ * reservation_object_add_shared_fence - Add a fence to a shared slot
+ * @obj: the reservation object
+ * @fence: the shared fence to add
+ *
  * Add a fence to a shared slot, obj->lock must be held, and
  * reservation_object_reserve_shared_fence has been called.
  */
@@ -204,6 +227,13 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
 }
 EXPORT_SYMBOL(reservation_object_add_shared_fence);
 
+/**
+ * reservation_object_add_excl_fence - Add an exclusive fence.
+ * @obj: the reservation object
+ * @fence: the shared fence to add
+ *
+ * Add a fence to the exclusive slot.  The obj->lock must be held.
+ */
 void reservation_object_add_excl_fence(struct reservation_object *obj,
 				       struct fence *fence)
 {
@@ -239,6 +269,18 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
 }
 EXPORT_SYMBOL(reservation_object_add_excl_fence);
 
+/**
+ * reservation_object_get_fences_rcu - Get an object's shared and exclusive
+ * fences without update side lock held
+ * @obj: the reservation object
+ * @pfence_excl: the returned exclusive fence (or NULL)
+ * @pshared_count: the number of shared fences returned
+ * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
+ * the required size, and must be freed by caller)
+ *
+ * RETURNS
+ * Zero or -errno
+ */
 int reservation_object_get_fences_rcu(struct reservation_object *obj,
 				      struct fence **pfence_excl,
 				      unsigned *pshared_count,
@@ -325,6 +367,18 @@ unlock:
 }
 EXPORT_SYMBOL_GPL(reservation_object_get_fences_rcu);
 
+/**
+ * reservation_object_wait_timeout_rcu - Wait on reservation's objects
+ * shared and/or exclusive fences.
+ * @obj: the reservation object
+ * @wait_all: if true, wait on all fences, else wait on just exclusive fence
+ * @intr: if true, do interruptible wait
+ * @timeout: timeout value in jiffies or zero to return immediately
+ *
+ * RETURNS
+ * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or
+ * greater than zer on success.
+ */
 long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
 					 bool wait_all, bool intr,
 					 unsigned long timeout)
@@ -422,6 +476,16 @@ reservation_object_test_signaled_single(struct fence *passed_fence)
 	return ret;
 }
 
+/**
+ * reservation_object_test_signaled_rcu - Test if a reservation object's
+ * fences have been signaled.
+ * @obj: the reservation object
+ * @test_all: if true, test all fences, otherwise only test the exclusive
+ * fence
+ *
+ * RETURNS
+ * true if all fences signaled, else false
+ */
 bool reservation_object_test_signaled_rcu(struct reservation_object *obj,
 					  bool test_all)
 {
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index 49d0576..b0f305e 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -49,12 +49,27 @@ extern struct ww_class reservation_ww_class;
 extern struct lock_class_key reservation_seqcount_class;
 extern const char reservation_seqcount_string[];
 
+/**
+ * struct reservation_object_list - a list of shared fences
+ * @rcu: for internal use
+ * @shared_count: table of shared fences
+ * @shared_max: for growing shared fence table
+ * @shared: shared fence table
+ */
 struct reservation_object_list {
 	struct rcu_head rcu;
 	u32 shared_count, shared_max;
 	struct fence __rcu *shared[];
 };
 
+/**
+ * struct reservation_object - a reservation object manages fences for a buffer
+ * @lock: update side lock
+ * @seq: sequence count for managing RCU read-side synchronization
+ * @fence_excl: the exclusive fence, if there is one currently
+ * @fence: list of current shared fences
+ * @staged: staged copy of shared fences for RCU updates
+ */
 struct reservation_object {
 	struct ww_mutex lock;
 	seqcount_t seq;
@@ -68,6 +83,10 @@ struct reservation_object {
 #define reservation_object_assert_held(obj) \
 	lockdep_assert_held(&(obj)->lock.base)
 
+/**
+ * reservation_object_init - initialize a reservation object
+ * @obj: the reservation object
+ */
 static inline void
 reservation_object_init(struct reservation_object *obj)
 {
@@ -79,6 +98,10 @@ reservation_object_init(struct reservation_object *obj)
 	obj->staged = NULL;
 }
 
+/**
+ * reservation_object_fini - destroys a reservation object
+ * @obj: the reservation object
+ */
 static inline void
 reservation_object_fini(struct reservation_object *obj)
 {
@@ -106,6 +129,14 @@ reservation_object_fini(struct reservation_object *obj)
 	ww_mutex_destroy(&obj->lock);
 }
 
+/**
+ * reservation_object_get_list - get the reservation object's
+ * shared fence list, with update-side lock held
+ * @obj: the reservation object
+ *
+ * Returns the shared fence list.  Does NOT take references to
+ * the fence.  The obj->lock must be held.
+ */
 static inline struct reservation_object_list *
 reservation_object_get_list(struct reservation_object *obj)
 {
@@ -113,6 +144,17 @@ reservation_object_get_list(struct reservation_object *obj)
 					 reservation_object_held(obj));
 }
 
+/**
+ * reservation_object_get_excl - get the reservation object's
+ * exclusive fence, with update-side lock held
+ * @obj: the reservation object
+ *
+ * Returns the exclusive fence (if any).  Does NOT take a
+ * reference.  The obj->lock must be held.
+ *
+ * RETURNS
+ * The exclusive fence or NULL
+ */
 static inline struct fence *
 reservation_object_get_excl(struct reservation_object *obj)
 {
@@ -120,6 +162,17 @@ reservation_object_get_excl(struct reservation_object *obj)
 					 reservation_object_held(obj));
 }
 
+/**
+ * reservation_object_get_excl_rcu - get the reservation object's
+ * exclusive fence, without lock held.
+ * @obj: the reservation object
+ *
+ * If there is an exclusive fence, this atomically increments it's
+ * reference count and returns it.
+ *
+ * RETURNS
+ * The exclusive fence or NULL if none
+ */
 static inline struct fence *
 reservation_object_get_excl_rcu(struct reservation_object *obj)
 {
-- 
2.5.5

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

[PATCH 4/4] doc: update/fixup dma-buf related DocBook

[Rob Clark]

Split out dma-buf related parts into their own section, add missing
files, and write a bit of overview about how it all fits together.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 Documentation/DocBook/device-drivers.tmpl | 36 +++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl
index cdd8b24..9c41af9 100644
--- a/Documentation/DocBook/device-drivers.tmpl
+++ b/Documentation/DocBook/device-drivers.tmpl
@@ -128,14 +128,42 @@ X!Edrivers/base/interface.c
 !Edrivers/base/platform.c
 !Edrivers/base/bus.c
      </sect1>
-     <sect1><title>Device Drivers DMA Management</title>
+     <sect1>
+       <title>Buffer Sharing and Synchronization</title>
+       <para>
+         The dma-buf subsystem provides the framework for sharing buffers
+         for hardware (DMA) access across multiple device drivers and
+         subsystems, and for synchronizing asynchronous hardware access.
+       </para>
+       <para>
+         This is used, for example, by drm "prime" multi-GPU support, but
+         is of course not limited to GPU use cases.
+       </para>
+       <para>
+         The three main components of this are: (1) dma-buf, representing
+         a sg_table and exposed to userspace as a file descriptor to allow
+         passing between devices, (2) fence, which provides a mechanism
+         to signal when one device as finished access, and (3) reservation,
+         which manages the shared or exclusive fence(s) associated with
+         the buffer.
+       </para>
+       <sect2><title>dma-buf</title>
 !Edrivers/dma-buf/dma-buf.c
+!Iinclude/linux/dma-buf.h
+       </sect2>
+       <sect2><title>reservation</title>
+!Pdrivers/dma-buf/reservation.c Reservation Object Overview
+!Edrivers/dma-buf/reservation.c
+!Iinclude/linux/reservation.h
+       </sect2>
+       <sect2><title>fence</title>
 !Edrivers/dma-buf/fence.c
-!Edrivers/dma-buf/seqno-fence.c
 !Iinclude/linux/fence.h
+!Edrivers/dma-buf/seqno-fence.c
 !Iinclude/linux/seqno-fence.h
-!Edrivers/dma-buf/reservation.c
-!Iinclude/linux/reservation.h
+       </sect2>
+     </sect1>
+     <sect1><title>Device Drivers DMA Management</title>
 !Edrivers/base/dma-coherent.c
 !Edrivers/base/dma-mapping.c
      </sect1>
-- 
2.5.5

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

Re: [PATCH 0/4] dma-buf/reservation doc updates

[Alex Deucher]

On Thu, Mar 31, 2016 at 4:26 PM, Rob Clark <robdclark@gmail.com> wrote:
> Some dma-buf headerdoc fixes (turns out dma-buf.h wasn't getting pulled
> into doc build), add reservation headerdoc, and fixup a bit the section
> in device-drivers.tmpl.
>
> Rob Clark (4):
>   reservation: sprinkle some WARN_ON()s
>   dma-buf: headerdoc fixes
>   reservation: add headerdoc comments
>   doc: update/fixup dma-buf related DocBook

For the series:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
>  Documentation/DocBook/device-drivers.tmpl | 36 ++++++++++++--
>  drivers/dma-buf/reservation.c             | 78 +++++++++++++++++++++++++++++--
>  include/linux/dma-buf.h                   | 13 ++++--
>  include/linux/reservation.h               | 53 +++++++++++++++++++++
>  4 files changed, 169 insertions(+), 11 deletions(-)
>
> --
> 2.5.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] dma-buf/reservation doc updates

[Sumit Semwal]

Hi Rob,

On 1 April 2016 at 19:37, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Mar 31, 2016 at 4:26 PM, Rob Clark <robdclark@gmail.com> wrote:
>> Some dma-buf headerdoc fixes (turns out dma-buf.h wasn't getting pulled
>> into doc build), add reservation headerdoc, and fixup a bit the section
>> in device-drivers.tmpl.
>>
Thanks for catching these! I'll queue them up into my for-next in the next week.
>> Rob Clark (4):
>>   reservation: sprinkle some WARN_ON()s
>>   dma-buf: headerdoc fixes
>>   reservation: add headerdoc comments
>>   doc: update/fixup dma-buf related DocBook
>
> For the series:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
>>
>>  Documentation/DocBook/device-drivers.tmpl | 36 ++++++++++++--
>>  drivers/dma-buf/reservation.c             | 78 +++++++++++++++++++++++++++++--
>>  include/linux/dma-buf.h                   | 13 ++++--
>>  include/linux/reservation.h               | 53 +++++++++++++++++++++
>>  4 files changed, 169 insertions(+), 11 deletions(-)
>>
>> --
>> 2.5.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 0/4] dma-buf/reservation doc updates

[Daniel Vetter]

On Fri, Apr 01, 2016 at 08:50:32PM +0530, Sumit Semwal wrote:
> Hi Rob,
> 
> On 1 April 2016 at 19:37, Alex Deucher <alexdeucher@gmail.com> wrote:
> > On Thu, Mar 31, 2016 at 4:26 PM, Rob Clark <robdclark@gmail.com> wrote:
> >> Some dma-buf headerdoc fixes (turns out dma-buf.h wasn't getting pulled
> >> into doc build), add reservation headerdoc, and fixup a bit the section
> >> in device-drivers.tmpl.
> >>
> Thanks for catching these! I'll queue them up into my for-next in the next week.

Since they failed to show up in 4.7 I've pulled them into drm-misc now.
Well except for patch 4, that one conflicts. And given that we'll have an
ongoing docbook->sphinx conversion probably better to redo after that
again.
-Daniel

> >> Rob Clark (4):
> >>   reservation: sprinkle some WARN_ON()s
> >>   dma-buf: headerdoc fixes
> >>   reservation: add headerdoc comments
> >>   doc: update/fixup dma-buf related DocBook
> >
> > For the series:
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> >
> >>
> >>  Documentation/DocBook/device-drivers.tmpl | 36 ++++++++++++--
> >>  drivers/dma-buf/reservation.c             | 78 +++++++++++++++++++++++++++++--
> >>  include/linux/dma-buf.h                   | 13 ++++--
> >>  include/linux/reservation.h               | 53 +++++++++++++++++++++
> >>  4 files changed, 169 insertions(+), 11 deletions(-)
> >>
> >> --
> >> 2.5.5
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> BR,
> Sumit.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] dma-buf/reservation doc updates

[Sumit Semwal]

[-- Attachment #1.1: Type: text/plain, Size: 2516 bytes --]

On 04-Jun-2016 5:51 PM, "Daniel Vetter" <daniel@ffwll.ch> wrote:
>
> On Fri, Apr 01, 2016 at 08:50:32PM +0530, Sumit Semwal wrote:
> > Hi Rob,
> >
> > On 1 April 2016 at 19:37, Alex Deucher <alexdeucher@gmail.com> wrote:
> > > On Thu, Mar 31, 2016 at 4:26 PM, Rob Clark <robdclark@gmail.com>
wrote:
> > >> Some dma-buf headerdoc fixes (turns out dma-buf.h wasn't getting
pulled
> > >> into doc build), add reservation headerdoc, and fixup a bit the
section
> > >> in device-drivers.tmpl.
> > >>
> > Thanks for catching these! I'll queue them up into my for-next in the
next week.
>
> Since they failed to show up in 4.7 I've pulled them into drm-misc now.
> Well except for patch 4, that one conflicts. And given that we'll have an
> ongoing docbook->sphinx conversion probably better to redo after that
> again.

Hi Daniel,

You should be able to see 3 of the patches (except the WARN_ON one, since
there was a build issue with that) in 4.7, as my pull request was pulled in
by Linus a couple of days ago. [1]

Best regards,
Sumit.

[1]:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ebb8cb2bae0344ef45cc173cdf5402ec91198abd
> -Daniel
>
> > >> Rob Clark (4):
> > >>   reservation: sprinkle some WARN_ON()s
> > >>   dma-buf: headerdoc fixes
> > >>   reservation: add headerdoc comments
> > >>   doc: update/fixup dma-buf related DocBook
> > >
> > > For the series:
> > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > >
> > >>
> > >>  Documentation/DocBook/device-drivers.tmpl | 36 ++++++++++++--
> > >>  drivers/dma-buf/reservation.c             | 78
+++++++++++++++++++++++++++++--
> > >>  include/linux/dma-buf.h                   | 13 ++++--
> > >>  include/linux/reservation.h               | 53 +++++++++++++++++++++
> > >>  4 files changed, 169 insertions(+), 11 deletions(-)
> > >>
> > >> --
> > >> 2.5.5
> > >>
> > >> _______________________________________________
> > >> dri-devel mailing list
> > >> dri-devel@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > BR,
> > Sumit.
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: Type: text/html, Size: 4168 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 0/4] dma-buf/reservation doc updates

[Daniel Vetter]

On Sun, Jun 5, 2016 at 1:17 PM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>
> On 04-Jun-2016 5:51 PM, "Daniel Vetter" <daniel@ffwll.ch> wrote:
>>
>> On Fri, Apr 01, 2016 at 08:50:32PM +0530, Sumit Semwal wrote:
>> > Hi Rob,
>> >
>> > On 1 April 2016 at 19:37, Alex Deucher <alexdeucher@gmail.com> wrote:
>> > > On Thu, Mar 31, 2016 at 4:26 PM, Rob Clark <robdclark@gmail.com>
>> > > wrote:
>> > >> Some dma-buf headerdoc fixes (turns out dma-buf.h wasn't getting
>> > >> pulled
>> > >> into doc build), add reservation headerdoc, and fixup a bit the
>> > >> section
>> > >> in device-drivers.tmpl.
>> > >>
>> > Thanks for catching these! I'll queue them up into my for-next in the
>> > next week.
>>
>> Since they failed to show up in 4.7 I've pulled them into drm-misc now.
>> Well except for patch 4, that one conflicts. And given that we'll have an
>> ongoing docbook->sphinx conversion probably better to redo after that
>> again.
>
> Hi Daniel,
>
> You should be able to see 3 of the patches (except the WARN_ON one, since
> there was a build issue with that) in 4.7, as my pull request was pulled in
> by Linus a couple of days ago. [1]

Oops, totally missed that since it wasn't in 4.7-rc1. So I assumed
that it didn't go in. Sorry for the mess :(
-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

Re: [PATCH 0/4] dma-buf/reservation doc updates

[Sumit Semwal]

On 5 June 2016 at 22:48, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sun, Jun 5, 2016 at 1:17 PM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>
>> On 04-Jun-2016 5:51 PM, "Daniel Vetter" <daniel@ffwll.ch> wrote:
>>>
>>> On Fri, Apr 01, 2016 at 08:50:32PM +0530, Sumit Semwal wrote:
>>> > Hi Rob,
>>> >
>>> > On 1 April 2016 at 19:37, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> > > On Thu, Mar 31, 2016 at 4:26 PM, Rob Clark <robdclark@gmail.com>
>>> > > wrote:
>>> > >> Some dma-buf headerdoc fixes (turns out dma-buf.h wasn't getting
>>> > >> pulled
>>> > >> into doc build), add reservation headerdoc, and fixup a bit the
>>> > >> section
>>> > >> in device-drivers.tmpl.
>>> > >>
>>> > Thanks for catching these! I'll queue them up into my for-next in the
>>> > next week.
>>>
>>> Since they failed to show up in 4.7 I've pulled them into drm-misc now.
>>> Well except for patch 4, that one conflicts. And given that we'll have an
>>> ongoing docbook->sphinx conversion probably better to redo after that
>>> again.
>>
>> Hi Daniel,
>>
>> You should be able to see 3 of the patches (except the WARN_ON one, since
>> there was a build issue with that) in 4.7, as my pull request was pulled in
>> by Linus a couple of days ago. [1]
>
> Oops, totally missed that since it wasn't in 4.7-rc1. So I assumed
> that it didn't go in. Sorry for the mess :(

Np at all, Daniel; you're right, these didn't go in for -rc1, and most
of it was cosmetic, so I requested a pull for -rc2.

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] reservation: sprinkle some WARN_ON()s

[Rob Clark]

A bit overkill since, for example, the rcu_dereference_protected() in
reservation_object_get_list() will WARN.  But this is much less subtle
for folks reading the code.

v2: use reservation_object_held() instead of ww_mutex_is_locked()

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/dma-buf/reservation.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index c0bd572..439af82 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
 	struct reservation_object_list *fobj, *old;
 	u32 max;
 
+	WARN_ON(!reservation_object_held(obj));
+
 	old = reservation_object_get_list(obj);
 
 	if (old && old->shared_max) {
@@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
 {
 	struct reservation_object_list *old, *fobj = obj->staged;
 
+	WARN_ON(!reservation_object_held(obj));
+
 	old = reservation_object_get_list(obj);
 	obj->staged = NULL;
 
@@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
 	struct reservation_object_list *old;
 	u32 i = 0;
 
+	WARN_ON(!reservation_object_held(obj));
+
 	old = reservation_object_get_list(obj);
 	if (old)
 		i = old->shared_count;
-- 
2.5.5

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

Re: [PATCH] reservation: sprinkle some WARN_ON()s

[Daniel Vetter]

On Thu, Apr 07, 2016 at 08:52:56PM -0400, Rob Clark wrote:
> A bit overkill since, for example, the rcu_dereference_protected() in
> reservation_object_get_list() will WARN.  But this is much less subtle
> for folks reading the code.
> 
> v2: use reservation_object_held() instead of ww_mutex_is_locked()
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Doesn't seem to build here:


In file included from ./arch/arm/include/asm/bug.h:59:0,
                 from include/linux/bug.h:4,
                 from include/linux/thread_info.h:11,
                 from include/asm-generic/current.h:4,
                 from arch/arm/include/generated/asm/current.h:1,
                 from include/linux/mutex.h:13,
                 from include/linux/ww_mutex.h:17,
                 from include/linux/reservation.h:42,
                 from drivers/dma-buf/reservation.c:35:
drivers/dma-buf/reservation.c: In function ‘reservation_object_reserve_shared’:
include/linux/reservation.h:82:38: error: implicit declaration of function ‘lockdep_is_held’ [-Werror=implicit-function-declaration]
 #define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base)
                                      ^
include/asm-generic/bug.h:92:25: note: in definition of macro ‘WARN_ON’
  int __ret_warn_on = !!(condition);    \
                         ^
drivers/dma-buf/reservation.c:74:11: note: in expansion of macro ‘reservation_object_held’
  WARN_ON(!reservation_object_held(obj));

Sounds like we should include some headers from reservation.h.
-Daniel

> ---
>  drivers/dma-buf/reservation.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index c0bd572..439af82 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
>  	struct reservation_object_list *fobj, *old;
>  	u32 max;
>  
> +	WARN_ON(!reservation_object_held(obj));
> +
>  	old = reservation_object_get_list(obj);
>  
>  	if (old && old->shared_max) {
> @@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
>  {
>  	struct reservation_object_list *old, *fobj = obj->staged;
>  
> +	WARN_ON(!reservation_object_held(obj));
> +
>  	old = reservation_object_get_list(obj);
>  	obj->staged = NULL;
>  
> @@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
>  	struct reservation_object_list *old;
>  	u32 i = 0;
>  
> +	WARN_ON(!reservation_object_held(obj));
> +
>  	old = reservation_object_get_list(obj);
>  	if (old)
>  		i = old->shared_count;
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] reservation: sprinkle some WARN_ON()s

[Rob Clark]

On Sat, Jun 4, 2016 at 8:52 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Apr 07, 2016 at 08:52:56PM -0400, Rob Clark wrote:
>> A bit overkill since, for example, the rcu_dereference_protected() in
>> reservation_object_get_list() will WARN.  But this is much less subtle
>> for folks reading the code.
>>
>> v2: use reservation_object_held() instead of ww_mutex_is_locked()
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>
> Doesn't seem to build here:
>
>
> In file included from ./arch/arm/include/asm/bug.h:59:0,
>                  from include/linux/bug.h:4,
>                  from include/linux/thread_info.h:11,
>                  from include/asm-generic/current.h:4,
>                  from arch/arm/include/generated/asm/current.h:1,
>                  from include/linux/mutex.h:13,
>                  from include/linux/ww_mutex.h:17,
>                  from include/linux/reservation.h:42,
>                  from drivers/dma-buf/reservation.c:35:
> drivers/dma-buf/reservation.c: In function ‘reservation_object_reserve_shared’:
> include/linux/reservation.h:82:38: error: implicit declaration of function ‘lockdep_is_held’ [-Werror=implicit-function-declaration]
>  #define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base)
>                                       ^
> include/asm-generic/bug.h:92:25: note: in definition of macro ‘WARN_ON’
>   int __ret_warn_on = !!(condition);    \
>                          ^
> drivers/dma-buf/reservation.c:74:11: note: in expansion of macro ‘reservation_object_held’
>   WARN_ON(!reservation_object_held(obj));
>
> Sounds like we should include some headers from reservation.h.


Sumit hit something similar.. not sure if he figured out the problem.
But seemed to only work if lockdep was enabled, iirc?  It is probably
true that I didn't try building without lockdep..  although I don't
remember kbuild robot complaining.  I'm a bit confused since
reservation_object_held() was used before.

Hmm, on second thought, without lockdep, rcu_lockdep_assert() macro is
a no-op, so the use of reservation_object_held() goes away..  :-/

Maybe we should just go back to earlier version of the patch which
used mutex_is_locked() instead?

BR,
-R

> -Daniel
>
>> ---
>>  drivers/dma-buf/reservation.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
>> index c0bd572..439af82 100644
>> --- a/drivers/dma-buf/reservation.c
>> +++ b/drivers/dma-buf/reservation.c
>> @@ -52,6 +52,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj)
>>       struct reservation_object_list *fobj, *old;
>>       u32 max;
>>
>> +     WARN_ON(!reservation_object_held(obj));
>> +
>>       old = reservation_object_get_list(obj);
>>
>>       if (old && old->shared_max) {
>> @@ -189,6 +191,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj,
>>  {
>>       struct reservation_object_list *old, *fobj = obj->staged;
>>
>> +     WARN_ON(!reservation_object_held(obj));
>> +
>>       old = reservation_object_get_list(obj);
>>       obj->staged = NULL;
>>
>> @@ -207,6 +211,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
>>       struct reservation_object_list *old;
>>       u32 i = 0;
>>
>> +     WARN_ON(!reservation_object_held(obj));
>> +
>>       old = reservation_object_get_list(obj);
>>       if (old)
>>               i = old->shared_count;
>> --
>> 2.5.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel