On Thu, Apr 09, 2015 at 04:24:53PM +0100, Chris Wilson wrote:
> On Thu, Apr 09, 2015 at 05:02:36PM +0200, Daniel Vetter wrote:
> > On Tue, Apr 07, 2015 at 04:20:51PM +0100, Chris Wilson wrote:
> > > @@ -640,7 +641,7 @@ static int logical_ring_wait_request(struct 
> > > intel_ringbuffer *ringbuf,
> > >                   break;
> > >   }
> > >  
> > > - if (&request->list == &ring->request_list)
> > > + if (WARN_ON(&request->list == &ring->request_list))
> > >           return -ENOSPC;
> > 
> > Checking for new_space < n (and initializing new_space to 0) would be a
> > clearer check imo. But that's just a bikeshed. Same for the legacy one
> > below.
> 
> If you watch later, I remove the double update of ringbuf->space.
> However, I am quite found of the if (iter == list_head) return -ENOSPC,
> so I am a bit biased.

Oh it was mostly that I had to double-check the loop above (which was out
of the diff context). With context it's all good. I'm a really lazy
reviewer ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to