[PATCH] staging/android: add TODO to de-stage android sync framework

[PATCH] staging/android: add TODO to de-stage android sync framework

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

 - remove sw_sync, it is used only for testing/debugging and should not
be upstreamed.
 - port sw_sync testcases to use debugfs somehow
 - clean up and ABI check for security issues
 - move the sync framework to drivers/base/dma-buf

Cc: Arve Hjønnevåg <arve@android.com>
Cc: Riley Andrews <riandrews@android.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Greg Hackmann <ghackmann@google.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/TODO | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 8f3ac37..2375dae 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -25,5 +25,12 @@ ion/
    exposes existing cma regions and doesn't reserve unecessarily memory when
    booting a system which doesn't use ion.
 
+sync framework:
+ - remove sw_sync, it is used only for testing/debugging and should not be
+upstreamed.
+ - port sw_sync testcases to use debugfs somehow
+ - clean up and ABI check for security issues
+ - move it to drivers/base/dma-buf
+
 Please send patches to Greg Kroah-Hartman <greg@kroah.com> and Cc:
 Arve Hjønnevåg <arve@android.com> and Riley Andrews <riandrews@android.com>
-- 
2.1.0

Re: [PATCH] staging/android: add TODO to de-stage android sync framework

[Daniel Vetter]

On Mon, Nov 23, 2015 at 05:41:53PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
>  - remove sw_sync, it is used only for testing/debugging and should not
> be upstreamed.
>  - port sw_sync testcases to use debugfs somehow
>  - clean up and ABI check for security issues
>  - move the sync framework to drivers/base/dma-buf
> 
> Cc: Arve Hjønnevåg <arve@android.com>
> Cc: Riley Andrews <riandrews@android.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Greg Hackmann <ghackmann@google.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

This reflects my recollection of various discussions at conferences and on
irc. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/staging/android/TODO | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
> index 8f3ac37..2375dae 100644
> --- a/drivers/staging/android/TODO
> +++ b/drivers/staging/android/TODO
> @@ -25,5 +25,12 @@ ion/
>     exposes existing cma regions and doesn't reserve unecessarily memory when
>     booting a system which doesn't use ion.
>  
> +sync framework:
> + - remove sw_sync, it is used only for testing/debugging and should not be
> +upstreamed.
> + - port sw_sync testcases to use debugfs somehow
> + - clean up and ABI check for security issues
> + - move it to drivers/base/dma-buf
> +
>  Please send patches to Greg Kroah-Hartman <greg@kroah.com> and Cc:
>  Arve Hjønnevåg <arve@android.com> and Riley Andrews <riandrews@android.com>
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] staging/android: add TODO to de-stage android sync framework

[Daniel Vetter]

On Tue, Nov 24, 2015 at 09:51:12AM +0100, Daniel Vetter wrote:
> On Mon, Nov 23, 2015 at 05:41:53PM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> >  - remove sw_sync, it is used only for testing/debugging and should not
> > be upstreamed.
> >  - port sw_sync testcases to use debugfs somehow
> >  - clean up and ABI check for security issues
> >  - move the sync framework to drivers/base/dma-buf
> > 
> > Cc: Arve Hjønnevåg <arve@android.com>
> > Cc: Riley Andrews <riandrews@android.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Greg Hackmann <ghackmann@google.com>
> > Cc: John Harrison <John.C.Harrison@Intel.com>
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> This reflects my recollection of various discussions at conferences and on
> irc. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Coffee just kicked in ;-)

> > ---
> >  drivers/staging/android/TODO | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
> > index 8f3ac37..2375dae 100644
> > --- a/drivers/staging/android/TODO
> > +++ b/drivers/staging/android/TODO
> > @@ -25,5 +25,12 @@ ion/
> >     exposes existing cma regions and doesn't reserve unecessarily memory when
> >     booting a system which doesn't use ion.
> >  
> > +sync framework:
> > + - remove sw_sync, it is used only for testing/debugging and should not be
> > +upstreamed.
> > + - port sw_sync testcases to use debugfs somehow

With all the effort going on around kselftest it'd be good to integrate
the existing testsuite google has into upstream too. Should probably be
listed here too.
-Daniel

> > + - clean up and ABI check for security issues
> > + - move it to drivers/base/dma-buf
> > +
> >  Please send patches to Greg Kroah-Hartman <greg@kroah.com> and Cc:
> >  Arve Hjønnevåg <arve@android.com> and Riley Andrews <riandrews@android.com>
> > -- 
> > 2.1.0
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] staging/android: add TODO to de-stage android sync framework

[Greg Hackmann]

On 11/24/2015 12:53 AM, Daniel Vetter wrote:
> With all the effort going on around kselftest it'd be good to integrate
> the existing testsuite google has into upstream too. Should probably be
> listed here too.
> -Daniel

The test code's available in AOSP:

https://android.googlesource.com/platform/system/core/+/master/libsync/tests/

Be warned that it sits on top of a small helper library, uses C++ 
heavily, and depends on googletest.  So it's going to need reworking 
before it's suitable for the kernel tree.  But you can at least see the 
kinds of things it's testing (and where the SW_SYNC_USER parts fit into 
the picture).

Re: [PATCH] staging/android: add TODO to de-stage android sync framework

[Greg Hackmann]

On 11/23/15 11:41 AM, Gustavo Padovan wrote:
> + - remove sw_sync, it is used only for testing/debugging and should not be
> +upstreamed.
> + - port sw_sync testcases to use debugfs somehow

A quick but important nitpick:

sw_sync itself is just an in-kernel helper for creating fences, when you 
don't have something like sync timeline primitives baked into your hardware.

CONFIG_SW_SYNC_USER adds the interface for creating and signaling 
sw_sync objects from userspace.  This is the part that's dangerous and 
only intended for testing, etc.

AFAIK CONFIG_SW_SYNC_USER is the only part people have been objecting 
to.  I'm fine with removing it.  Removing the kernel-facing side of 
sw_sync would be a problem for us, since many drivers use it to create 
their fences.

Re: [PATCH] staging/android: add TODO to de-stage android sync framework

[Gustavo Padovan]

2015-11-24 Greg Hackmann <ghackmann@google.com>:

> On 11/23/15 11:41 AM, Gustavo Padovan wrote:
> >+ - remove sw_sync, it is used only for testing/debugging and should not be
> >+upstreamed.
> >+ - port sw_sync testcases to use debugfs somehow
> 
> A quick but important nitpick:
> 
> sw_sync itself is just an in-kernel helper for creating fences, when you
> don't have something like sync timeline primitives baked into your hardware.
> 
> CONFIG_SW_SYNC_USER adds the interface for creating and signaling sw_sync
> objects from userspace.  This is the part that's dangerous and only intended
> for testing, etc.
> 
> AFAIK CONFIG_SW_SYNC_USER is the only part people have been objecting to.
> I'm fine with removing it.  Removing the kernel-facing side of sw_sync would
> be a problem for us, since many drivers use it to create their fences.

Right, I probably misundertood things, I'm okay with removing only
CONFIG_SW_SYNC_USER and if others are okay too I'll just send an updated
patch for the TODO.

	Gustavo