Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp

[Gustavo Padovan <gustavo@padovan.org>]

2017-02-14 Chris Wilson <chris@chris-wilson.co.uk>:

> On Tue, Feb 14, 2017 at 11:40:38AM -0200, Gustavo Padovan wrote:
> > Hi Chris,
> > 
> > 2017-02-14 Chris Wilson <chris@chris-wilson.co.uk>:
> > > diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> > > index c769dc653b34..bfead12390f2 100644
> > > --- a/drivers/dma-buf/sync_debug.c
> > > +++ b/drivers/dma-buf/sync_debug.c
> > > @@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
> > >  		   show ? "_" : "",
> > >  		   sync_status_str(status));
> > >  
> > > -	if (status) {
> > > +	if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) {
> > >  		struct timespec64 ts64 =
> > >  			ktime_to_timespec64(fence->timestamp);
> > 
> > How about add this test_bit() to dma_fence_is_signaled_locked() so
> > we test both for DMA_FENCE_FLAG_SIGNALED_BIT and
> > DMA_FENCE_FLAG_TIMESTAMP_BIT there at the same time?
> 
> I was thinking of only using it as communication with the timestamp
> user. That avoids getting into the situation as to which bit truly means
> is-signaled and we still only synchronize on SIGNALED_BIT.
> 
> It would be possible, but I don't think it makes anything simpler.

Yes, it doesn't make anything better. We should keep it that way for
users that doesn't need timestamp.

> 
> One thing that occurs to me is whether we should be setting the
> timestamp when we set an error. The above (sync_debug though) implies
> that it expects the error to have the timestamp. sync_fence_info could
> go either way.

We could do it. I don't see any reason against it.

Gustavo

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx