Re: [Intel-gfx] [RFC PATCH 35/97] drm/i915/guc: Improve error message for unsolicited CT response

2021-05-25 Thread Matthew Brost
On Mon, May 24, 2021 at 01:59:54PM +0200, Michal Wajdeczko wrote:
> 
> 
> On 06.05.2021 21:13, Matthew Brost wrote:
> > Improve the error message when a unsolicited CT response is received by
> > printing fence that couldn't be found, the last fence, and all requests
> > with a response outstanding.
> > 
> > Signed-off-by: Matthew Brost 
> > ---
> >  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 10 +++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c 
> > b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> > index 217ab3ebd1af..a76603537fa8 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> > @@ -703,12 +703,16 @@ static int ct_handle_response(struct intel_guc_ct 
> > *ct, struct ct_incoming_msg *r
> > found = true;
> > break;
> > }
> > -   spin_unlock_irqrestore(>requests.lock, flags);
> > -
> > if (!found) {
> > CT_ERROR(ct, "Unsolicited response (fence %u)\n", fence);
> > -   return -ENOKEY;
> > +   CT_ERROR(ct, "Could not find fence=%u, last_fence=%u\n", fence,
> > +ct->requests.last_fence);
> 
> nit: this new wording may suggest that it's our fault, but that's not
> necessary true
> 

I don't think is implies whos fault this is either way.

> > +   list_for_each_entry(req, >requests.pending, link)
> > +   CT_ERROR(ct, "request %u awaits response\n",
> > +req->fence);
> 
> usually we don't send multiple requests that expects responses, so it's
> very likely that list with pending requests will be empty, and even if
> list is not empty, I'm not sure what is the relation between those
> pending requests to this unsolicited response, thus wondering how these
> extra errors could improve our debugging experience ?
> 

The more information when this occurs the better.

Matt

> > +   err = -ENOKEY;
> > }
> > +   spin_unlock_irqrestore(>requests.lock, flags);
> >  
> > if (unlikely(err))
> > return err;
> > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [RFC PATCH 35/97] drm/i915/guc: Improve error message for unsolicited CT response

2021-05-24 Thread Michal Wajdeczko



On 06.05.2021 21:13, Matthew Brost wrote:
> Improve the error message when a unsolicited CT response is received by
> printing fence that couldn't be found, the last fence, and all requests
> with a response outstanding.
> 
> Signed-off-by: Matthew Brost 
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> index 217ab3ebd1af..a76603537fa8 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> @@ -703,12 +703,16 @@ static int ct_handle_response(struct intel_guc_ct *ct, 
> struct ct_incoming_msg *r
>   found = true;
>   break;
>   }
> - spin_unlock_irqrestore(>requests.lock, flags);
> -
>   if (!found) {
>   CT_ERROR(ct, "Unsolicited response (fence %u)\n", fence);
> - return -ENOKEY;
> + CT_ERROR(ct, "Could not find fence=%u, last_fence=%u\n", fence,
> +  ct->requests.last_fence);

nit: this new wording may suggest that it's our fault, but that's not
necessary true

> + list_for_each_entry(req, >requests.pending, link)
> + CT_ERROR(ct, "request %u awaits response\n",
> +  req->fence);

usually we don't send multiple requests that expects responses, so it's
very likely that list with pending requests will be empty, and even if
list is not empty, I'm not sure what is the relation between those
pending requests to this unsolicited response, thus wondering how these
extra errors could improve our debugging experience ?

> + err = -ENOKEY;
>   }
> + spin_unlock_irqrestore(>requests.lock, flags);
>  
>   if (unlikely(err))
>   return err;
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx