| Message ID | 20210506191451.77768-36-matthew.brost@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Basic GuC submission support in the i915 | expand |
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 <matthew.brost@intel.com> > --- > 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(&ct->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, &ct->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(&ct->requests.lock, flags); > > if (unlikely(err)) > return err; >
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 <matthew.brost@intel.com> > > --- > > 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(&ct->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, &ct->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(&ct->requests.lock, flags); > > > > if (unlikely(err)) > > return err; > >
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(&ct->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); + list_for_each_entry(req, &ct->requests.pending, link) + CT_ERROR(ct, "request %u awaits response\n", + req->fence); + err = -ENOKEY; } + spin_unlock_irqrestore(&ct->requests.lock, flags); if (unlikely(err)) return err;
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 <matthew.brost@intel.com> --- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)