| 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
| ||
|
Message-ID: <20140304081411.GK17001@phenom.ffwll.local>
Date: Tue, 4 Mar 2014 09:14:11 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: Maarten Lankhorst <maarten.lankhorst@...onical.com>
Cc: Daniel Vetter <daniel@...ll.ch>, Ian Lister <ian.lister@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-arch@...r.kernel.org, Colin Cross <ccross@...gle.com>,
"linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
"Clark, Rob" <robdclark@...il.com>,
dri-devel <dri-devel@...ts.freedesktop.org>,
Sumit Semwal <sumit.semwal@...aro.org>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>
Subject: Re: [PATCH 4/6] android: convert sync to fence api, v4
On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:
> op 03-03-14 22:11, Daniel Vetter schreef:
> >On Mon, Feb 17, 2014 at 04:57:19PM +0100, Maarten Lankhorst wrote:
> >>Android syncpoints can be mapped to a timeline. This removes the need
> >>to maintain a separate api for synchronization. I've left the android
> >>trace events in place, but the core fence events should already be
> >>sufficient for debugging.
> >>
> >>v2:
> >>- Call fence_remove_callback in sync_fence_free if not all fences have fired.
> >>v3:
> >>- Merge Colin Cross' bugfixes, and the android fence merge optimization.
> >>v4:
> >>- Merge with the upstream fixes.
> >>
> >>Signed-off-by: Maarten Lankhorst <maarten.lankhorst@...onical.com>
> >>---
> >Snipped everything but headers - Ian Lister from our android team is
> >signed up to have a more in-depth look at proper integration with android
> >syncpoints. Adding him to cc.
> >
> >>diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
> >>index 62e2255b1c1e..6036dbdc8e6f 100644
> >>--- a/drivers/staging/android/sync.h
> >>+++ b/drivers/staging/android/sync.h
> >>@@ -21,6 +21,7 @@
> >> #include <linux/list.h>
> >> #include <linux/spinlock.h>
> >> #include <linux/wait.h>
> >>+#include <linux/fence.h>
> >>
> >> struct sync_timeline;
> >> struct sync_pt;
> >>@@ -40,8 +41,6 @@ struct sync_fence;
> >> * -1 if a will signal before b
> >> * @free_pt: called before sync_pt is freed
> >> * @release_obj: called before sync_timeline is freed
> >>- * @print_obj: deprecated
> >>- * @print_pt: deprecated
> >> * @fill_driver_data: write implementation specific driver data to data.
> >> * should return an error if there is not enough room
> >> * as specified by size. This information is returned
> >>@@ -67,13 +66,6 @@ struct sync_timeline_ops {
> >> /* optional */
> >> void (*release_obj)(struct sync_timeline *sync_timeline);
> >>
> >>- /* deprecated */
> >>- void (*print_obj)(struct seq_file *s,
> >>- struct sync_timeline *sync_timeline);
> >>-
> >>- /* deprecated */
> >>- void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt);
> >>-
> >> /* optional */
> >> int (*fill_driver_data)(struct sync_pt *syncpt, void *data, int size);
> >>
> >>@@ -104,42 +96,48 @@ struct sync_timeline {
> >>
> >> /* protected by child_list_lock */
> >> bool destroyed;
> >>+ int context, value;
> >>
> >> struct list_head child_list_head;
> >> spinlock_t child_list_lock;
> >>
> >> struct list_head active_list_head;
> >>- spinlock_t active_list_lock;
> >>
> >>+#ifdef CONFIG_DEBUG_FS
> >> struct list_head sync_timeline_list;
> >>+#endif
> >> };
> >>
> >> /**
> >> * struct sync_pt - sync point
> >>- * @parent: sync_timeline to which this sync_pt belongs
> >>+ * @fence: base fence class
> >> * @child_list: membership in sync_timeline.child_list_head
> >> * @active_list: membership in sync_timeline.active_list_head
> >>+<<<<<<< current
> >> * @signaled_list: membership in temporary signaled_list on stack
> >> * @fence: sync_fence to which the sync_pt belongs
> >> * @pt_list: membership in sync_fence.pt_list_head
> >> * @status: 1: signaled, 0:active, <0: error
> >> * @timestamp: time which sync_pt status transitioned from active to
> >> * signaled or error.
> >>+=======
> >>+>>>>>>> patched
> >Conflict markers ...
> Oops.
> >> */
> >> struct sync_pt {
> >>- struct sync_timeline *parent;
> >>- struct list_head child_list;
> >>+ struct fence base;
> >Hm, embedding feels wrong, since that still means that I'll need to
> >implement two kinds of fences in i915 - one using the seqno fence to make
> >dma-buf sync work, and one to implmenent sync_pt to make the android folks
> >happy.
> >
> >If I can dream I think we should have a pointer to an underlying fence
> >here, i.e. a struct sync_pt would just be a userspace interface wrapper to
> >do explicit syncing using native fences, instead of implicit syncing like
> >with dma-bufs. But this is all drive-by comments from a very cursory
> >high-level look. I might be full of myself again ;-)
> >-Daniel
> >
> No, the idea is that because android syncpoint is simply another type of
> dma-fence, that if you deal with normal fences then android can
> automatically be handled too. The userspace fence api android exposes
> could be very easily made to work for dma-fence, just pass a dma-fence
> to sync_fence_create.
> So exposing dma-fence would probably work for android too.
Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists