Re: [Intel-gfx] [PATCH 3/6] drm/i915: Update ring position from request on retiring
Quoting Mika Kuoppala (2018-03-09 13:38:37) > Chris Wilsonwrites: > > > When wedged, we do not update the ring->tail as we submit the requests > > causing us to leak the ring->space upon cleaning up the wedged driver. > > We can just use the value stored in rq->tail, and keep the submission > > backend details away from set-wedge. > > > > Signed-off-by: Chris Wilson > > Cc: Mika Kuoppala > > --- > > drivers/gpu/drm/i915/i915_request.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index efa9ee557f31..69b378a323fc 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -358,7 +358,7 @@ static void advance_ring(struct i915_request *request) > >* is just about to be. Either works, if we miss the last two > >* noops - they are safe to be replayed on a reset. > >*/ > > - tail = READ_ONCE(request->ring->tail); > > + tail = READ_ONCE(request->tail); > > I tried to think if we need the READ_ONCE here anymore. I tried as well to see if the comment was still correct. It still is due to that we can retire before we see the context-switch interrupt. > But as this is the safest version, > Reviewed-by: Mika Kuoppala > > Noticed that request->tail is not cleared on i915_request_alloc. > > If we would set rq->head = rq->tail = rq->ring->emit > we could use rq->head == rq->tail to assert that > we screw up something major during the request lifetime. Heh, if we get a stray write here, we're going to get stray writes everywhere :) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Update ring position from request on retiring
Chris Wilsonwrites: > When wedged, we do not update the ring->tail as we submit the requests > causing us to leak the ring->space upon cleaning up the wedged driver. > We can just use the value stored in rq->tail, and keep the submission > backend details away from set-wedge. > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_request.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c > b/drivers/gpu/drm/i915/i915_request.c > index efa9ee557f31..69b378a323fc 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -358,7 +358,7 @@ static void advance_ring(struct i915_request *request) >* is just about to be. Either works, if we miss the last two >* noops - they are safe to be replayed on a reset. >*/ > - tail = READ_ONCE(request->ring->tail); > + tail = READ_ONCE(request->tail); I tried to think if we need the READ_ONCE here anymore. But as this is the safest version, Reviewed-by: Mika Kuoppala Noticed that request->tail is not cleared on i915_request_alloc. If we would set rq->head = rq->tail = rq->ring->emit we could use rq->head == rq->tail to assert that we screw up something major during the request lifetime. -Mika > } else { > tail = request->postfix; > } > -- > 2.16.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915: Update ring position from request on retiring
When wedged, we do not update the ring->tail as we submit the requests causing us to leak the ring->space upon cleaning up the wedged driver. We can just use the value stored in rq->tail, and keep the submission backend details away from set-wedge. Signed-off-by: Chris WilsonCc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index efa9ee557f31..69b378a323fc 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -358,7 +358,7 @@ static void advance_ring(struct i915_request *request) * is just about to be. Either works, if we miss the last two * noops - they are safe to be replayed on a reset. */ - tail = READ_ONCE(request->ring->tail); + tail = READ_ONCE(request->tail); } else { tail = request->postfix; } -- 2.16.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx