From: Gustavo Padovan <[email protected]>
- 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 <[email protected]>
Cc: Riley Andrews <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Greg Hackmann <[email protected]>
Cc: John Harrison <[email protected]>
Signed-off-by: Gustavo Padovan <[email protected]>
---
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 <[email protected]> and Cc:
Arve Hjønnevåg <[email protected]> and Riley Andrews <[email protected]>
--
2.1.0
On Mon, Nov 23, 2015 at 05:41:53PM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> - 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 <[email protected]>
> Cc: Riley Andrews <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Rob Clark <[email protected]>
> Cc: Greg Hackmann <[email protected]>
> Cc: John Harrison <[email protected]>
> Signed-off-by: Gustavo Padovan <[email protected]>
This reflects my recollection of various discussions at conferences and on
irc. Acked-by: Daniel Vetter <[email protected]>
> ---
> 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 <[email protected]> and Cc:
> Arve Hj?nnev?g <[email protected]> and Riley Andrews <[email protected]>
> --
> 2.1.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
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 <[email protected]>
> >
> > - 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 <[email protected]>
> > Cc: Riley Andrews <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Cc: Rob Clark <[email protected]>
> > Cc: Greg Hackmann <[email protected]>
> > Cc: John Harrison <[email protected]>
> > Signed-off-by: Gustavo Padovan <[email protected]>
>
> This reflects my recollection of various discussions at conferences and on
> irc. Acked-by: Daniel Vetter <[email protected]>
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 <[email protected]> and Cc:
> > Arve Hj?nnev?g <[email protected]> and Riley Andrews <[email protected]>
> > --
> > 2.1.0
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
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.
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).
2015-11-24 Greg Hackmann <[email protected]>:
> 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