lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAO_48GHsfKfo2ozH=C=y8bJozvtO7ZuX3f_VUzC4XzGp1Mgu=Q@mail.gmail.com>
Date:	Thu, 28 Jul 2016 16:30:36 +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>,
	Stéphane Marchesin <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

Hi Greg,

On 19 July 2016 at 17:51, Sumit Semwal <sumit.semwal@...aro.org> wrote:
> (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?

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@...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.
>>
Best,
Sumit.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ