[PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in struct sync_file

2016-07-28 Thread Sumit Semwal 
Hi Greg,

On 19 July 2016 at 17:51, Sumit Semwal  wrote:
> (Adding Greg KH)
>
> Hi Greg,
>
> On 19 July 2016 at 17:45, Sumit Semwal  wrote:
>> Hi Greg,
>>
>>
>> On 12 July 2016 at 23:38, Gustavo Padovan  wrote:
>>> From: Gustavo Padovan 
>>>
>>> Create sync_file->fence to abstract the type of fence we are using for
>>> each sync_file. If only one fence is present we use a normal struct fence
>>> but if there is more fences to be added to the sync_file a fence_array
>>> is created.
>>>
>>> This change cleans up sync_file a bit. We don't need to have sync_file_cb
>>> array anymore. Instead, as we always have  one fence, only one fence
>>> callback is registered per sync_file.
>>>
>> Since this is a simple change in sync_debug,c, may I request for your
>> Ack so I could take it along with the other dma-buf patches?
>>
> Missed the fact that you weren't CCed; for this simple update to
> sync_debug,c, may I request for your Ack so I can take it with dam-buf
> patches?

Gentle reminder please: since it's a small change, if you could Ack
it, I'd be happy to take it along with the dma-buf patches and queue
it up.

>>> v4: fixes checkpatch warnings
>>>
>>> v4: Comments from Chris Wilson
>>> - use sizeof(*fence) to reallocate array
>>> - fix typo in comments
>>> - protect num_fences sum against overflows
>>> - use array->base instead of casting the to struct fence
>>>
>>> v3: Comments from Chris Wilson and Christian König
>>> - struct sync_file lost status member in favor of 
>>> fence_is_signaled()
>>> - drop use of fence_array_teardown()
>>> - use sizeof(*fence) to allocate only an array on fence pointers
>>>
>>> v2: Comments from Chris Wilson and Christian König
>>> - Not using fence_ops anymore
>>> - fence_is_array() was created to differentiate fence from 
>>> fence_array
>>> - fence_array_teardown() is now exported and used under 
>>> fence_is_array()
>>> - struct sync_file lost num_fences member
>>>
>>> Cc: Chris Wilson 
>>> Cc: Christian König 
>>> Signed-off-by: Gustavo Padovan 
>>> Reviewed-by: Chris Wilson 
>>> Acked-by: Christian König 
>>> ---
>>>  drivers/dma-buf/sync_file.c  | 169 
>>> +++
>>>  drivers/staging/android/sync_debug.c |  12 ++-
>>>  include/linux/sync_file.h|  17 ++--
>>>  3 files changed, 124 insertions(+), 74 deletions(-)
>>>
>> 
>>
>>> diff --git a/drivers/staging/android/sync_debug.c 
>>> b/drivers/staging/android/sync_debug.c
>>> index 5f57499..e07958c 100644
>>> --- a/drivers/staging/android/sync_debug.c
>>> +++ b/drivers/staging/android/sync_debug.c
>>> @@ -159,10 +159,16 @@ static void sync_print_sync_file(struct seq_file *s,
>>> int i;
>>>
>>> seq_printf(s, "[%p] %s: %s\n", sync_file, sync_file->name,
>>> -  sync_status_str(atomic_read(_file->status)));
>>> +  sync_status_str(!fence_is_signaled(sync_file->fence)));
>>>
>>> -   for (i = 0; i < sync_file->num_fences; ++i)
>>> -   sync_print_fence(s, sync_file->cbs[i].fence, true);
>>> +   if (fence_is_array(sync_file->fence)) {
>>> +   struct fence_array *array = 
>>> to_fence_array(sync_file->fence);
>>> +
>>> +   for (i = 0; i < array->num_fences; ++i)
>>> +   sync_print_fence(s, array->fences[i], true);
>>> +   } else {
>>> +   sync_print_fence(s, sync_file->fence, true);
>>> +   }
>>>  }
>>>
>>>  static int sync_debugfs_show(struct seq_file *s, void *unused)
>>> diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
>>> index c6ffe8b..2efc5ec 100644
>>> --- a/include/linux/sync_file.h
>>> +++ b/include/linux/sync_file.h
>>> @@ -19,12 +19,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> -
>>> -struct sync_file_cb {
>>> -   struct fence_cb cb;
>>> -   struct fence *fence;
>>> -   struct sync_file *sync_file;
>>> -};
>>> +#include 
>>>
>>>  /**
>>>   * struct sync_file - sync file to export to the userspace
>>> @@ -32,10 +27,9 @@ struct sync_file_cb {
>>>   * @kref:  reference count on fence.
>>>   * @name:  name of sync_file.  Useful for debugging
>>>   * @sync_file_list:membership in global file list
>>> - * @num_fences:number of sync_pts in the fence
>>>   * @wq:wait queue for fence signaling
>>> - * @status:0: signaled, >0:active, <0: error
>>> - * @cbs:   sync_pts callback information
>>> + * @fence: fence with the fences in the sync_file
>>> + * @cb:fence callback information
>>>   */
>>>  struct sync_file {
>>> struct file *file;
>>> @@ -44,12 +38,11 @@ struct sync_file {
>>>  #ifdef CONFIG_DEBUG_FS
>>> struct list_headsync_file_list;
>>>  #endif
>>> -   int num_fences;
>>>
>>> wait_queue_head_t   wq;
>>> -   atomic_tstatus;

[PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in struct sync_file

2016-07-28 Thread Greg Kroah-Hartman 
On Thu, Jul 28, 2016 at 04:30:36PM +0530, Sumit Semwal wrote:
> Hi Greg,
> 
> On 19 July 2016 at 17:51, Sumit Semwal  wrote:
> > (Adding Greg KH)
> >
> > Hi Greg,
> >
> > On 19 July 2016 at 17:45, Sumit Semwal  wrote:
> >> Hi Greg,
> >>
> >>
> >> On 12 July 2016 at 23:38, Gustavo Padovan  wrote:
> >>> From: Gustavo Padovan 
> >>>
> >>> Create sync_file->fence to abstract the type of fence we are using for
> >>> each sync_file. If only one fence is present we use a normal struct fence
> >>> but if there is more fences to be added to the sync_file a fence_array
> >>> is created.
> >>>
> >>> This change cleans up sync_file a bit. We don't need to have sync_file_cb
> >>> array anymore. Instead, as we always have  one fence, only one fence
> >>> callback is registered per sync_file.
> >>>
> >> Since this is a simple change in sync_debug,c, may I request for your
> >> Ack so I could take it along with the other dma-buf patches?
> >>
> > Missed the fact that you weren't CCed; for this simple update to
> > sync_debug,c, may I request for your Ack so I can take it with dam-buf
> > patches?
> 
> Gentle reminder please: since it's a small change, if you could Ack
> it, I'd be happy to take it along with the dma-buf patches and queue
> it up.

Ugh, sorry, vacation and travel and merge window hasn't been good to me
this cycle...

This is fine with me, please take it through your tree if you want to,
and again, sorry for the delay in responding.  Usually, for staging
bits, if you want/need to merge something due to an api change, no need
to wait for me, I can handle any merge conflicts / issues that might
come up after the fact.

Acked-by: Greg Kroah-Hartman 

thanks,

greg k-h

[PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in struct sync_file

2016-07-19 Thread Sumit Semwal 
(Adding Greg KH)

Hi Greg,

On 19 July 2016 at 17:45, Sumit Semwal  wrote:
> Hi Greg,
>
>
> On 12 July 2016 at 23:38, Gustavo Padovan  wrote:
>> From: Gustavo Padovan 
>>
>> Create sync_file->fence to abstract the type of fence we are using for
>> each sync_file. If only one fence is present we use a normal struct fence
>> but if there is more fences to be added to the sync_file a fence_array
>> is created.
>>
>> This change cleans up sync_file a bit. We don't need to have sync_file_cb
>> array anymore. Instead, as we always have  one fence, only one fence
>> callback is registered per sync_file.
>>
> Since this is a simple change in sync_debug,c, may I request for your
> Ack so I could take it along with the other dma-buf patches?
>
Missed the fact that you weren't CCed; for this simple update to
sync_debug,c, may I request for your Ack so I can take it with dam-buf
patches?
>> v4: fixes checkpatch warnings
>>
>> v4: Comments from Chris Wilson
>> - use sizeof(*fence) to reallocate array
>> - fix typo in comments
>> - protect num_fences sum against overflows
>> - use array->base instead of casting the to struct fence
>>
>> v3: Comments from Chris Wilson and Christian König
>> - struct sync_file lost status member in favor of fence_is_signaled()
>> - drop use of fence_array_teardown()
>> - use sizeof(*fence) to allocate only an array on fence pointers
>>
>> v2: Comments from Chris Wilson and Christian König
>> - Not using fence_ops anymore
>> - fence_is_array() was created to differentiate fence from 
>> fence_array
>> - fence_array_teardown() is now exported and used under 
>> fence_is_array()
>> - struct sync_file lost num_fences member
>>
>> Cc: Chris Wilson 
>> Cc: Christian König 
>> Signed-off-by: Gustavo Padovan 
>> Reviewed-by: Chris Wilson 
>> Acked-by: Christian König 
>> ---
>>  drivers/dma-buf/sync_file.c  | 169 
>> +++
>>  drivers/staging/android/sync_debug.c |  12 ++-
>>  include/linux/sync_file.h|  17 ++--
>>  3 files changed, 124 insertions(+), 74 deletions(-)
>>
> 
>
>> diff --git a/drivers/staging/android/sync_debug.c 
>> b/drivers/staging/android/sync_debug.c
>> index 5f57499..e07958c 100644
>> --- a/drivers/staging/android/sync_debug.c
>> +++ b/drivers/staging/android/sync_debug.c
>> @@ -159,10 +159,16 @@ static void sync_print_sync_file(struct seq_file *s,
>> int i;
>>
>> seq_printf(s, "[%p] %s: %s\n", sync_file, sync_file->name,
>> -  sync_status_str(atomic_read(_file->status)));
>> +  sync_status_str(!fence_is_signaled(sync_file->fence)));
>>
>> -   for (i = 0; i < sync_file->num_fences; ++i)
>> -   sync_print_fence(s, sync_file->cbs[i].fence, true);
>> +   if (fence_is_array(sync_file->fence)) {
>> +   struct fence_array *array = to_fence_array(sync_file->fence);
>> +
>> +   for (i = 0; i < array->num_fences; ++i)
>> +   sync_print_fence(s, array->fences[i], true);
>> +   } else {
>> +   sync_print_fence(s, sync_file->fence, true);
>> +   }
>>  }
>>
>>  static int sync_debugfs_show(struct seq_file *s, void *unused)
>> diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
>> index c6ffe8b..2efc5ec 100644
>> --- a/include/linux/sync_file.h
>> +++ b/include/linux/sync_file.h
>> @@ -19,12 +19,7 @@
>>  #include 
>>  #include 
>>  #include 
>> -
>> -struct sync_file_cb {
>> -   struct fence_cb cb;
>> -   struct fence *fence;
>> -   struct sync_file *sync_file;
>> -};
>> +#include 
>>
>>  /**
>>   * struct sync_file - sync file to export to the userspace
>> @@ -32,10 +27,9 @@ struct sync_file_cb {
>>   * @kref:  reference count on fence.
>>   * @name:  name of sync_file.  Useful for debugging
>>   * @sync_file_list:membership in global file list
>> - * @num_fences:number of sync_pts in the fence
>>   * @wq:wait queue for fence signaling
>> - * @status:0: signaled, >0:active, <0: error
>> - * @cbs:   sync_pts callback information
>> + * @fence: fence with the fences in the sync_file
>> + * @cb:fence callback information
>>   */
>>  struct sync_file {
>> struct file *file;
>> @@ -44,12 +38,11 @@ struct sync_file {
>>  #ifdef CONFIG_DEBUG_FS
>> struct list_headsync_file_list;
>>  #endif
>> -   int num_fences;
>>
>> wait_queue_head_t   wq;
>> -   atomic_tstatus;
>>
>> -   struct sync_file_cb cbs[];
>> +   struct fence*fence;
>> +   struct fence_cb cb;
>>  };
>>
>>  struct sync_file *sync_file_create(struct fence *fence);
>> --
>> 2.5.5
>>
>
> BR,
> ~Sumit.
>
> --
> Thanks and regards,
>
> Sumit Semwal
> Linaro Mobile Group - Kernel Team Lead
> Linaro.org │ Open source

[PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in struct sync_file

2016-07-19 Thread Sumit Semwal 
Hi Greg,


On 12 July 2016 at 23:38, Gustavo Padovan  wrote:
> From: Gustavo Padovan 
>
> Create sync_file->fence to abstract the type of fence we are using for
> each sync_file. If only one fence is present we use a normal struct fence
> but if there is more fences to be added to the sync_file a fence_array
> is created.
>
> This change cleans up sync_file a bit. We don't need to have sync_file_cb
> array anymore. Instead, as we always have  one fence, only one fence
> callback is registered per sync_file.
>
Since this is a simple change in sync_debug,c, may I request for your
Ack so I could take it along with the other dma-buf patches?

> v4: fixes checkpatch warnings
>
> v4: Comments from Chris Wilson
> - use sizeof(*fence) to reallocate array
> - fix typo in comments
> - protect num_fences sum against overflows
> - use array->base instead of casting the to struct fence
>
> v3: Comments from Chris Wilson and Christian König
> - struct sync_file lost status member in favor of fence_is_signaled()
> - drop use of fence_array_teardown()
> - use sizeof(*fence) to allocate only an array on fence pointers
>
> v2: Comments from Chris Wilson and Christian König
> - Not using fence_ops anymore
> - fence_is_array() was created to differentiate fence from fence_array
> - fence_array_teardown() is now exported and used under 
> fence_is_array()
> - struct sync_file lost num_fences member
>
> Cc: Chris Wilson 
> Cc: Christian König 
> Signed-off-by: Gustavo Padovan 
> Reviewed-by: Chris Wilson 
> Acked-by: Christian König 
> ---
>  drivers/dma-buf/sync_file.c  | 169 
> +++
>  drivers/staging/android/sync_debug.c |  12 ++-
>  include/linux/sync_file.h|  17 ++--
>  3 files changed, 124 insertions(+), 74 deletions(-)
>


> diff --git a/drivers/staging/android/sync_debug.c 
> b/drivers/staging/android/sync_debug.c
> index 5f57499..e07958c 100644
> --- a/drivers/staging/android/sync_debug.c
> +++ b/drivers/staging/android/sync_debug.c
> @@ -159,10 +159,16 @@ static void sync_print_sync_file(struct seq_file *s,
> int i;
>
> seq_printf(s, "[%p] %s: %s\n", sync_file, sync_file->name,
> -  sync_status_str(atomic_read(_file->status)));
> +  sync_status_str(!fence_is_signaled(sync_file->fence)));
>
> -   for (i = 0; i < sync_file->num_fences; ++i)
> -   sync_print_fence(s, sync_file->cbs[i].fence, true);
> +   if (fence_is_array(sync_file->fence)) {
> +   struct fence_array *array = to_fence_array(sync_file->fence);
> +
> +   for (i = 0; i < array->num_fences; ++i)
> +   sync_print_fence(s, array->fences[i], true);
> +   } else {
> +   sync_print_fence(s, sync_file->fence, true);
> +   }
>  }
>
>  static int sync_debugfs_show(struct seq_file *s, void *unused)
> diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
> index c6ffe8b..2efc5ec 100644
> --- a/include/linux/sync_file.h
> +++ b/include/linux/sync_file.h
> @@ -19,12 +19,7 @@
>  #include 
>  #include 
>  #include 
> -
> -struct sync_file_cb {
> -   struct fence_cb cb;
> -   struct fence *fence;
> -   struct sync_file *sync_file;
> -};
> +#include 
>
>  /**
>   * struct sync_file - sync file to export to the userspace
> @@ -32,10 +27,9 @@ struct sync_file_cb {
>   * @kref:  reference count on fence.
>   * @name:  name of sync_file.  Useful for debugging
>   * @sync_file_list:membership in global file list
> - * @num_fences:number of sync_pts in the fence
>   * @wq:wait queue for fence signaling
> - * @status:0: signaled, >0:active, <0: error
> - * @cbs:   sync_pts callback information
> + * @fence: fence with the fences in the sync_file
> + * @cb:fence callback information
>   */
>  struct sync_file {
> struct file *file;
> @@ -44,12 +38,11 @@ struct sync_file {
>  #ifdef CONFIG_DEBUG_FS
> struct list_headsync_file_list;
>  #endif
> -   int num_fences;
>
> wait_queue_head_t   wq;
> -   atomic_tstatus;
>
> -   struct sync_file_cb cbs[];
> +   struct fence*fence;
> +   struct fence_cb cb;
>  };
>
>  struct sync_file *sync_file_create(struct fence *fence);
> --
> 2.5.5
>

BR,
~Sumit.

-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs

[PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in struct sync_file

2016-07-12 Thread Gustavo Padovan 
From: Gustavo Padovan 

Create sync_file->fence to abstract the type of fence we are using for
each sync_file. If only one fence is present we use a normal struct fence
but if there is more fences to be added to the sync_file a fence_array
is created.

This change cleans up sync_file a bit. We don't need to have sync_file_cb
array anymore. Instead, as we always have  one fence, only one fence
callback is registered per sync_file.

v4: fixes checkpatch warnings

v4: Comments from Chris Wilson
- use sizeof(*fence) to reallocate array
- fix typo in comments
- protect num_fences sum against overflows
- use array->base instead of casting the to struct fence

v3: Comments from Chris Wilson and Christian König
- struct sync_file lost status member in favor of fence_is_signaled()
- drop use of fence_array_teardown()
- use sizeof(*fence) to allocate only an array on fence pointers

v2: Comments from Chris Wilson and Christian König
- Not using fence_ops anymore
- fence_is_array() was created to differentiate fence from fence_array
- fence_array_teardown() is now exported and used under fence_is_array()
- struct sync_file lost num_fences member

Cc: Chris Wilson 
Cc: Christian König 
Signed-off-by: Gustavo Padovan 
Reviewed-by: Chris Wilson 
Acked-by: Christian König 
---
 drivers/dma-buf/sync_file.c  | 169 +++
 drivers/staging/android/sync_debug.c |  12 ++-
 include/linux/sync_file.h|  17 ++--
 3 files changed, 124 insertions(+), 74 deletions(-)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 9aaa608..a223f48 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -28,11 +28,11 @@

 static const struct file_operations sync_file_fops;

-static struct sync_file *sync_file_alloc(int size)
+static struct sync_file *sync_file_alloc(void)
 {
struct sync_file *sync_file;

-   sync_file = kzalloc(size, GFP_KERNEL);
+   sync_file = kzalloc(sizeof(*sync_file), GFP_KERNEL);
if (!sync_file)
return NULL;

@@ -45,6 +45,8 @@ static struct sync_file *sync_file_alloc(int size)

init_waitqueue_head(_file->wq);

+   INIT_LIST_HEAD(_file->cb.node);
+
return sync_file;

 err:
@@ -54,14 +56,11 @@ err:

 static void fence_check_cb_func(struct fence *f, struct fence_cb *cb)
 {
-   struct sync_file_cb *check;
struct sync_file *sync_file;

-   check = container_of(cb, struct sync_file_cb, cb);
-   sync_file = check->sync_file;
+   sync_file = container_of(cb, struct sync_file, cb);

-   if (atomic_dec_and_test(_file->status))
-   wake_up_all(_file->wq);
+   wake_up_all(_file->wq);
 }

 /**
@@ -76,22 +75,18 @@ struct sync_file *sync_file_create(struct fence *fence)
 {
struct sync_file *sync_file;

-   sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1]));
+   sync_file = sync_file_alloc();
if (!sync_file)
return NULL;

-   sync_file->num_fences = 1;
-   atomic_set(_file->status, 1);
+   sync_file->fence = fence;
+
snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d",
 fence->ops->get_driver_name(fence),
 fence->ops->get_timeline_name(fence), fence->context,
 fence->seqno);

-   sync_file->cbs[0].fence = fence;
-   sync_file->cbs[0].sync_file = sync_file;
-   if (fence_add_callback(fence, _file->cbs[0].cb,
-  fence_check_cb_func))
-   atomic_dec(_file->status);
+   fence_add_callback(fence, _file->cb, fence_check_cb_func);

return sync_file;
 }
@@ -121,14 +116,49 @@ err:
return NULL;
 }

-static void sync_file_add_pt(struct sync_file *sync_file, int *i,
-struct fence *fence)
+static int sync_file_set_fence(struct sync_file *sync_file,
+  struct fence **fences, int num_fences)
+{
+   struct fence_array *array;
+
+   /*
+* The reference for the fences in the new sync_file and held
+* in add_fence() during the merge procedure, so for num_fences == 1
+* we already own a new reference to the fence. For num_fence > 1
+* we own the reference of the fence_array creation.
+*/
+   if (num_fences == 1) {
+   sync_file->fence = fences[0];
+   } else {
+   array = fence_array_create(num_fences, fences,
+  fence_context_alloc(1), 1, false);
+   if (!array)
+   return -ENOMEM;
+
+   sync_file->fence = >base;
+   }
+
+   return 0;
+}
+
+static struct fence **get_fences(struct sync_file *sync_file, int *num_fences)
 {
-   sync_file->cbs[*i].fence = fence;
-   sync_file->cbs[*i].sync_file = sync_file;