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


Groups > linux.kernel > #1451877 > unrolled thread

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

Started bySumit Semwal <sumit.semwal@linaro.org>
First post2016-07-28 13:10 +0200
Last post2016-07-28 16:30 +0200
Articles 2 — 2 participants

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: [PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in  struct sync_file Sumit Semwal <sumit.semwal@linaro.org> - 2016-07-28 13:10 +0200
    Re: [PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in  struct sync_file Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-07-28 16:30 +0200

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

FromSumit Semwal <sumit.semwal@linaro.org>
Date2016-07-28 13:10 +0200
SubjectRe: [PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in struct sync_file
Message-ID<rZRmp-5r9-7@gated-at.bofh.it>
Hi Greg,

On 19 July 2016 at 17:51, Sumit Semwal <sumit.semwal@linaro.org> wrote:
> (Adding Greg KH)
>
> Hi Greg,
>
> On 19 July 2016 at 17:45, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>> Hi Greg,
>>
>>
>> On 12 July 2016 at 23:38, Gustavo Padovan <gustavo@padovan.org> wrote:
>>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>>
>>> 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 <chris@chris-wilson.co.uk>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Acked-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>  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(-)
>>>
>> <snip>
>>
>>> 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(&sync_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 <linux/list.h>
>>>  #include <linux/spinlock.h>
>>>  #include <linux/fence.h>
>>> -
>>> -struct sync_file_cb {
>>> -       struct fence_cb cb;
>>> -       struct fence *fence;
>>> -       struct sync_file *sync_file;
>>> -};
>>> +#include <linux/fence-array.h>
>>>
>>>  /**
>>>   * 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_head        sync_file_list;
>>>  #endif
>>> -       int num_fences;
>>>
>>>         wait_queue_head_t       wq;
>>> -       atomic_t                status;
>>>
>>> -       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.
>>
Best,
Sumit.

[toc] | [next] | [standalone]


#1451958

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2016-07-28 16:30 +0200
Message-ID<rZUtY-7qU-13@gated-at.bofh.it>
In reply to#1451877
On Thu, Jul 28, 2016 at 04:30:36PM +0530, Sumit Semwal wrote:
> Hi Greg,
> 
> On 19 July 2016 at 17:51, Sumit Semwal <sumit.semwal@linaro.org> wrote:
> > (Adding Greg KH)
> >
> > Hi Greg,
> >
> > On 19 July 2016 at 17:45, Sumit Semwal <sumit.semwal@linaro.org> wrote:
> >> Hi Greg,
> >>
> >>
> >> On 12 July 2016 at 23:38, Gustavo Padovan <gustavo@padovan.org> wrote:
> >>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >>>
> >>> 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 <gregkh@linuxfoundation.org>

thanks,

greg k-h

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web