[PATCH 0/7] de-stage SW_SYNC validation frawework

[Pavel Machek]

On Tue 2016-08-09 08:04:54, Daniel Vetter wrote:
> On Sun, Jul 24, 2016 at 05:00:31PM +0200, Pavel Machek wrote:
> > On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
> > > 2016-08-07 Pavel Machek :
> > > 
> > > > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
> > > > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > Do you think there is time to get this in for 4.8?
> > > > > 
> > > > > No, it was too late on my end, due to travel and vacation, sorry.  
> > > > > I'll
> > > > > queue it up for 4.9-rc1.
> > > > 
> > > > Could we get some documentation what this does? Is it visilble to
> > > > userspace?
> > > 
> > > This interface is only intended for testing and validation, there are
> > > ioctls on the debugfs file that can be accessed by userspace but there
> > > isn't any exported kernel header with this info. The tester should know
> > > and add a internal header to be able to access it. We want to prevent
> > > people from misusing this feature by not advertising it nor providing
> > > documentation.
> > 
> > You are playing dangerous game here. debugfs is not normally considered 
> > stable,
> > but otoh... ioctls on debugfs?
> 
> It's not considered stable. The idea is that we also add the existing
> testcases to kselftest. It's purely a bit of interface to be able to drive
> run the test logic for real fences. What it really tests is the fence
> interface (which is public in the uapi headers and all that), but to be
> able to do that we need some (hw-independent way) to expose fences, which
> this provides.
> 
> Long term we might even do this as a proper interface (with some
> restrictions to make it safe and avoid userspace pulling the kernel over
> the table). And then rip out sw_sync entirely.
> 
> Imo there's no need at all for docs for this.

There's full directory of files, with absolutely zero comments/documentation. 
They
are quite hard to understand. Plus it has userland interface.

IMO comment should be added, explaining what it is testing, that interface is 
not considered
stable, and where the test lives.

I know what "fence" is in the cpu sense (mfence and friends), but I'm not sure 
what "real fence" is in this context.

Best regards,

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Daniel Vetter]

On Sun, Jul 24, 2016 at 05:00:31PM +0200, Pavel Machek wrote:
> On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
> > 2016-08-07 Pavel Machek :
> > 
> > > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
> > > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
> > > > > Hi,
> > > > > 
> > > > > Do you think there is time to get this in for 4.8?
> > > > 
> > > > No, it was too late on my end, due to travel and vacation, sorry.  I'll
> > > > queue it up for 4.9-rc1.
> > > 
> > > Could we get some documentation what this does? Is it visilble to
> > > userspace?
> > 
> > This interface is only intended for testing and validation, there are
> > ioctls on the debugfs file that can be accessed by userspace but there
> > isn't any exported kernel header with this info. The tester should know
> > and add a internal header to be able to access it. We want to prevent
> > people from misusing this feature by not advertising it nor providing
> > documentation.
> 
> You are playing dangerous game here. debugfs is not normally considered 
> stable,
> but otoh... ioctls on debugfs?

It's not considered stable. The idea is that we also add the existing
testcases to kselftest. It's purely a bit of interface to be able to drive
run the test logic for real fences. What it really tests is the fence
interface (which is public in the uapi headers and all that), but to be
able to do that we need some (hw-independent way) to expose fences, which
this provides.

Long term we might even do this as a proper interface (with some
restrictions to make it safe and avoid userspace pulling the kernel over
the table). And then rip out sw_sync entirely.

Imo there's no need at all for docs for this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Pavel Machek]

On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
> 2016-08-07 Pavel Machek :
> 
> > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
> > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
> > > > Hi,
> > > > 
> > > > Do you think there is time to get this in for 4.8?
> > > 
> > > No, it was too late on my end, due to travel and vacation, sorry.  I'll
> > > queue it up for 4.9-rc1.
> > 
> > Could we get some documentation what this does? Is it visilble to
> > userspace?
> 
> This interface is only intended for testing and validation, there are
> ioctls on the debugfs file that can be accessed by userspace but there
> isn't any exported kernel header with this info. The tester should know
> and add a internal header to be able to access it. We want to prevent
> people from misusing this feature by not advertising it nor providing
> documentation.

You are playing dangerous game here. debugfs is not normally considered stable,
but otoh... ioctls on debugfs?

Anyway, please provide some documentation. Kernel hackers need to know what 
this does.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Gustavo Padovan]

2016-07-24 Pavel Machek :

> On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
> > 2016-08-07 Pavel Machek :
> > 
> > > On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
> > > > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
> > > > > Hi,
> > > > > 
> > > > > Do you think there is time to get this in for 4.8?
> > > > 
> > > > No, it was too late on my end, due to travel and vacation, sorry.  I'll
> > > > queue it up for 4.9-rc1.
> > > 
> > > Could we get some documentation what this does? Is it visilble to
> > > userspace?
> > 
> > This interface is only intended for testing and validation, there are
> > ioctls on the debugfs file that can be accessed by userspace but there
> > isn't any exported kernel header with this info. The tester should know
> > and add a internal header to be able to access it. We want to prevent
> > people from misusing this feature by not advertising it nor providing
> > documentation.
> 
> You are playing dangerous game here. debugfs is not normally considered 
> stable,
> but otoh... ioctls on debugfs?
> 
> Anyway, please provide some documentation. Kernel hackers need to know what 
> this does.

Okay, where do you think is the best place? Would documentation inside
the .c file suffice for you?

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Gustavo Padovan]

2016-08-07 Pavel Machek :

> On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
> > On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
> > > Hi,
> > > 
> > > Do you think there is time to get this in for 4.8?
> > 
> > No, it was too late on my end, due to travel and vacation, sorry.  I'll
> > queue it up for 4.9-rc1.
> 
> Could we get some documentation what this does? Is it visilble to
> userspace?

This interface is only intended for testing and validation, there are
ioctls on the debugfs file that can be accessed by userspace but there
isn't any exported kernel header with this info. The tester should know
and add a internal header to be able to access it. We want to prevent
people from misusing this feature by not advertising it nor providing
documentation.

There will be, though, kselftest for this interface, that I send out
once SW_SYNC is de-staged.

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Pavel Machek]

On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
> On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
> > Hi,
> > 
> > Do you think there is time to get this in for 4.8?
> 
> No, it was too late on my end, due to travel and vacation, sorry.  I'll
> queue it up for 4.9-rc1.

Could we get some documentation what this does? Is it visilble to
userspace?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Greg Kroah-Hartman]

On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
> Hi,
> 
> Do you think there is time to get this in for 4.8?

No, it was too late on my end, due to travel and vacation, sorry.  I'll
queue it up for 4.9-rc1.

thanks,

greg k-h

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Gustavo Padovan]

Hi,

Do you think there is time to get this in for 4.8?

Thanks.

2016-06-20 Gustavo Padovan :

> From: Gustavo Padovan 
> 
> Hi Greg,
> 
> This is the last step in the Sync Framwork de-stage task. It de-stage
> the SW_SYNC validation framework and the sync_debug info debugfs file.
> 
> The first 3 patches are clean up and improvements and the rest is preparation
> to de-stage and then finally the actual de-stage.
> 
> Please review,
> 
> Gustavo
> 
> ---
> Gustavo Padovan (7):
>   staging/android: move trace/sync.h to sync_trace.h
>   staging/android: remove doc from sw_sync
>   staging/android: display sync_pt name on debugfs
>   staging/android: do not let userspace trigger WARN_ON
>   staging/android: prepare sw_sync files for de-staging
>   dma-buf/sw_sync: de-stage SW_SYNC
>   staging/android: remove sync framework TODO
> 
>  drivers/dma-buf/Kconfig| 14 +++
>  drivers/dma-buf/Makefile   |  1 +
>  drivers/{staging/android => dma-buf}/sw_sync.c | 46 
> +++---
>  drivers/{staging/android => dma-buf}/sync_debug.c  |  7 ++--
>  drivers/{staging/android => dma-buf}/sync_debug.h  | 26 +---
>  .../android/trace/sync.h => dma-buf/sync_trace.h}  |  6 +--
>  drivers/staging/android/Kconfig| 13 --
>  drivers/staging/android/Makefile   |  1 -
>  drivers/staging/android/TODO   |  8 
>  9 files changed, 38 insertions(+), 84 deletions(-)
>  rename drivers/{staging/android => dma-buf}/sw_sync.c (84%)
>  rename drivers/{staging/android => dma-buf}/sync_debug.c (97%)
>  rename drivers/{staging/android => dma-buf}/sync_debug.h (72%)
>  rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)
> 
> -- 
> 2.5.5
> 

Gustavo

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Gustavo Padovan]

Hi Greg,

Any comment on this?

Thanks,

Gustavo

2016-06-20 Gustavo Padovan :

> From: Gustavo Padovan 
> 
> Hi Greg,
> 
> This is the last step in the Sync Framwork de-stage task. It de-stage
> the SW_SYNC validation framework and the sync_debug info debugfs file.
> 
> The first 3 patches are clean up and improvements and the rest is preparation
> to de-stage and then finally the actual de-stage.
> 
> Please review,
> 
> Gustavo
> 
> ---
> Gustavo Padovan (7):
>   staging/android: move trace/sync.h to sync_trace.h
>   staging/android: remove doc from sw_sync
>   staging/android: display sync_pt name on debugfs
>   staging/android: do not let userspace trigger WARN_ON
>   staging/android: prepare sw_sync files for de-staging
>   dma-buf/sw_sync: de-stage SW_SYNC
>   staging/android: remove sync framework TODO
> 
>  drivers/dma-buf/Kconfig| 14 +++
>  drivers/dma-buf/Makefile   |  1 +
>  drivers/{staging/android => dma-buf}/sw_sync.c | 46 
> +++---
>  drivers/{staging/android => dma-buf}/sync_debug.c  |  7 ++--
>  drivers/{staging/android => dma-buf}/sync_debug.h  | 26 +---
>  .../android/trace/sync.h => dma-buf/sync_trace.h}  |  6 +--
>  drivers/staging/android/Kconfig| 13 --
>  drivers/staging/android/Makefile   |  1 -
>  drivers/staging/android/TODO   |  8 
>  9 files changed, 38 insertions(+), 84 deletions(-)
>  rename drivers/{staging/android => dma-buf}/sw_sync.c (84%)
>  rename drivers/{staging/android => dma-buf}/sync_debug.c (97%)
>  rename drivers/{staging/android => dma-buf}/sync_debug.h (72%)
>  rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)
> 
> -- 
> 2.5.5
> 

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Pavel Machek]

Hi!

> From: Gustavo Padovan 
> 
> Hi Greg,
> 
> This is the last step in the Sync Framwork de-stage task. It

Typo: "fram_e_work"

> de-stage
> the SW_SYNC validation framework and the sync_debug info debugfs file.
> 
> The first 3 patches are clean up and improvements and the rest is preparation
> to de-stage and then finally the actual de-stage.

Could we get some kind of description what the sync framework does?
There are no useful comments in /sw_sync.c. There's no Documentation/
files. I don't know what this is supposed to do...

Pavel

> Please review,
> 
> Gustavo
> 
> ---
> Gustavo Padovan (7):
>   staging/android: move trace/sync.h to sync_trace.h
>   staging/android: remove doc from sw_sync
>   staging/android: display sync_pt name on debugfs
>   staging/android: do not let userspace trigger WARN_ON
>   staging/android: prepare sw_sync files for de-staging
>   dma-buf/sw_sync: de-stage SW_SYNC
>   staging/android: remove sync framework TODO
> 
>  drivers/dma-buf/Kconfig| 14 +++
>  drivers/dma-buf/Makefile   |  1 +
>  drivers/{staging/android => dma-buf}/sw_sync.c | 46 
> +++---
>  drivers/{staging/android => dma-buf}/sync_debug.c  |  7 ++--
>  drivers/{staging/android => dma-buf}/sync_debug.h  | 26 +---
>  .../android/trace/sync.h => dma-buf/sync_trace.h}  |  6 +--
>  drivers/staging/android/Kconfig| 13 --
>  drivers/staging/android/Makefile   |  1 -
>  drivers/staging/android/TODO   |  8 
>  9 files changed, 38 insertions(+), 84 deletions(-)
>  rename drivers/{staging/android => dma-buf}/sw_sync.c (84%)
>  rename drivers/{staging/android => dma-buf}/sync_debug.c (97%)
>  rename drivers/{staging/android => dma-buf}/sync_debug.h (72%)
>  rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)
> 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 0/7] de-stage SW_SYNC validation frawework

[Gustavo Padovan]

From: Gustavo Padovan 

Hi Greg,

This is the last step in the Sync Framwork de-stage task. It de-stage
the SW_SYNC validation framework and the sync_debug info debugfs file.

The first 3 patches are clean up and improvements and the rest is preparation
to de-stage and then finally the actual de-stage.

Please review,

Gustavo

---
Gustavo Padovan (7):
  staging/android: move trace/sync.h to sync_trace.h
  staging/android: remove doc from sw_sync
  staging/android: display sync_pt name on debugfs
  staging/android: do not let userspace trigger WARN_ON
  staging/android: prepare sw_sync files for de-staging
  dma-buf/sw_sync: de-stage SW_SYNC
  staging/android: remove sync framework TODO

 drivers/dma-buf/Kconfig| 14 +++
 drivers/dma-buf/Makefile   |  1 +
 drivers/{staging/android => dma-buf}/sw_sync.c | 46 +++---
 drivers/{staging/android => dma-buf}/sync_debug.c  |  7 ++--
 drivers/{staging/android => dma-buf}/sync_debug.h  | 26 +---
 .../android/trace/sync.h => dma-buf/sync_trace.h}  |  6 +--
 drivers/staging/android/Kconfig| 13 --
 drivers/staging/android/Makefile   |  1 -
 drivers/staging/android/TODO   |  8 
 9 files changed, 38 insertions(+), 84 deletions(-)
 rename drivers/{staging/android => dma-buf}/sw_sync.c (84%)
 rename drivers/{staging/android => dma-buf}/sync_debug.c (97%)
 rename drivers/{staging/android => dma-buf}/sync_debug.h (72%)
 rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)

-- 
2.5.5