[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAO_48GHFXcVqBsc2qM+_Ao9wRgHgJdJn1D5-6W1v21-+ucY25A@mail.gmail.com>
Date: Tue, 19 Jul 2016 17:51:21 +0530
From: Sumit Semwal <sumit.semwal@...aro.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: DRI mailing list <dri-devel@...ts.freedesktop.org>,
LKML <linux-kernel@...r.kernel.org>,
Daniel Stone <daniels@...labora.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Rob Clark <robdclark@...il.com>,
Greg Hackmann <ghackmann@...gle.com>,
John Harrison <John.C.Harrison@...el.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Sean Paul <seanpaul@...gle.com>, marcheu@...gle.com,
Mauro Carvalho Chehab <m.chehab@...sung.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Gustavo Padovan <gustavo.padovan@...labora.co.uk>,
Chris Wilson <chris@...is-wilson.co.uk>,
Christian König <christian.koenig@....com>,
Gustavo Padovan <gustavo@...ovan.org>
Subject: Re: [PATCH v4 2/5] dma-buf/sync_file: refactor fence storage in
struct sync_file
(Adding Greg KH)
Hi Greg,
On 19 July 2016 at 17:45, Sumit Semwal <sumit.semwal@...aro.org> wrote:
> Hi Greg,
>
>
> On 12 July 2016 at 23:38, Gustavo Padovan <gustavo@...ovan.org> wrote:
>> From: Gustavo Padovan <gustavo.padovan@...labora.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?
>> 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@...is-wilson.co.uk>
>> Cc: Christian König <christian.koenig@....com>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
>> Reviewed-by: Chris Wilson <chris@...is-wilson.co.uk>
>> Acked-by: Christian König <christian.koenig@....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.
>
> --
> Thanks and regards,
>
> Sumit Semwal
> Linaro Mobile Group - Kernel Team Lead
> Linaro.org │ Open source software for ARM SoCs
--
Thanks and regards,
Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
Powered by blists - more mailing lists