On Mon, Aug 16, 2021 at 06:51:19AM -0700, Matthew Brost wrote:
> A small race that could result in incorrect accounting of the number
> of outstanding G2H. Basically prior to this patch we did not increment
> the number of outstanding G2H if we encoutered a GT reset while sending
> a H2G. This was incorrect as the context state had already been updated
> to anticipate a G2H response thus the counter should be incremented.
>
> Fixes: f4eb1f3fe946 ("drm/i915/guc: Ensure G2H response has space in buffer")
> Signed-off-by: Matthew Brost <matthew.br...@intel.com>
> Cc: <sta...@vger.kernel.org>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 69faa39da178..b5d3972ae164 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -360,11 +360,13 @@ static int guc_submission_send_busy_loop(struct
> intel_guc *guc,
> {
> int err;
>
> - err = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
> -
> - if (!err && g2h_len_dw)
> + if (g2h_len_dw)
> atomic_inc(&guc->outstanding_submission_g2h);
>
> + err = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
I'm majorly confused by the _busy_loop naming scheme, especially here.
Like "why do we want to send a busy loop comand to guc, this doesn't make
sense".
It seems like you're using _busy_loop as a suffix for "this is ok to be
called in atomic context". The linux kernel bikeshed for this is generally
_atomic() (or _in_atomic() or something like that). Would be good to
rename to make this slightly less confusing.
-Daniel
> + if (err == -EBUSY && g2h_len_dw)
> + atomic_dec(&guc->outstanding_submission_g2h);
> +
> return err;
> }
>
> --
> 2.32.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
