All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: "Tales A. Mendonça" <talesam@gmail.com>
Cc: <intel-xe@lists.freedesktop.org>,
	<daniele.ceraolospurio@intel.com>, <stuart.summers@intel.com>,
	<julia.filipchuk@intel.com>, <thomas.hellstrom@linux.intel.com>,
	<rodrigo.vivi@intel.com>, <jani.nikula@intel.com>,
	<navonjohnlukose@gmail.com>, <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v4 2/3] drm/xe: Log when a timed out TLB invalidation ack finally arrives
Date: Sun, 20 Sep 2026 22:30:02 -0700	[thread overview]
Message-ID: <arDA2pUYH7uoUpE2@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <aq214e+ia3nHVrKB@gsse-cloud1.jf.intel.com>

On Fri, Sep 18, 2026 at 03:06:25PM -0700, Matthew Brost wrote:
> On Thu, Sep 17, 2026 at 01:35:52PM -0300, Tales A. Mendonça wrote:
> > When a TLB invalidation fence times out we log the timeout, but if the
> > ack for that seqno later shows up there is no record of it, making it
> > impossible to tell from logs whether the ack was lost forever or merely
> > (very) late.
> > 
> > Track the most recent timed out seqno and log how late its ack arrives,
> > relative to both the original request and the moment the fence was
> > signaled with -ETIME.
> > 
> > On ARL with GuC 70.53.0 this shows the acks are never lost: they
> > consistently arrive ~2.3s after the request, tens of milliseconds after
> > the TDR has already signaled the fence:
> > 
> >   TLB invalidation fence timeout, seqno=10992 recv=10991
> >   TLB invalidation late ack: seqno=10992 recv=10992,
> >     request-to-ack=2314ms, timeout-to-ack=45ms
> > 
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
> > Signed-off-by: Tales A. Mendonça <talesam@gmail.com>
> 
> I would give this an RB but my colleagues have done a bunch of work on
> SIGID and I haven't been involved at all, so I need some help here.
> 

I discussed with my colleagues - the we agree the TLB component should
hook into SIGID for logging but we will do this as a follow up on top of
this change.

So this is:
Reviewed-by: Matthew Brost <matthew.brost@intel.com> 

> > ---
> >  drivers/gpu/drm/xe/xe_tlb_inval.c       | 19 +++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_tlb_inval_types.h | 17 +++++++++++++++++
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > index 7a0c04fac60..7047a347551 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > @@ -14,6 +14,7 @@
> >  #include "xe_guc_tlb_inval.h"
> >  #include "xe_mmio.h"
> >  #include "xe_pm.h"
> > +#include "xe_printk.h"
> >  #include "xe_tlb_inval.h"
> >  #include "xe_trace.h"
> >  
> > @@ -99,6 +100,11 @@ static void xe_tlb_inval_fence_timeout(struct work_struct *work)
> >  			fence->seqno, tlb_inval->seqno_recv);
> >  
> >  		timedout_seqno = fence->seqno;
> > +		if (!tlb_inval->timedout_seqno) {
> > +			tlb_inval->timedout_seqno = fence->seqno;
> > +			tlb_inval->timedout_inval_time = fence->inval_time;
> > +			tlb_inval->timedout_time = ktime_get();
> > +		}
> >  
> >  		fence->base.error = -ETIME;
> >  		xe_tlb_inval_fence_signal(fence);
> > @@ -227,6 +233,7 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval)
> >  	else
> >  		pending_seqno = tlb_inval->seqno - 1;
> >  	WRITE_ONCE(tlb_inval->seqno_recv, pending_seqno);
> > +	tlb_inval->timedout_seqno = 0;
> >  
> >  	list_for_each_entry_safe(fence, next,
> >  				 &tlb_inval->pending_fences, link)
> > @@ -454,6 +461,18 @@ void xe_tlb_inval_done_handler(struct xe_tlb_inval *tlb_inval, int seqno)
> >  
> >  	WRITE_ONCE(tlb_inval->seqno_recv, seqno);
> >  
> > +	if (tlb_inval->timedout_seqno &&
> > +	    xe_tlb_inval_seqno_past(tlb_inval, tlb_inval->timedout_seqno)) {
> > +		ktime_t now = ktime_get();
> > +
> > +		xe_warn(xe,
> > +			"TLB invalidation late ack: seqno=%d recv=%d, request-to-ack=%lldms, timeout-to-ack=%lldms",
> > +			tlb_inval->timedout_seqno, seqno,
> > +			ktime_ms_delta(now, tlb_inval->timedout_inval_time),
> > +			ktime_ms_delta(now, tlb_inval->timedout_time));
> 
> Should this be some type SIGID message?
> 
> Matt
> 
> > +		tlb_inval->timedout_seqno = 0;
> > +	}
> > +
> >  	list_for_each_entry_safe(fence, next,
> >  				 &tlb_inval->pending_fences, link) {
> >  		trace_xe_tlb_inval_fence_recv(xe, fence);
> > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_types.h b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > index d77be1aedc9..80d2019fa20 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > @@ -112,6 +112,23 @@ struct xe_tlb_inval {
> >  	 * @pending_lock: protects @pending_fences and updating @seqno_recv.
> >  	 */
> >  	spinlock_t pending_lock;
> > +	/**
> > +	 * @timedout_seqno: seqno of the most recent timed out TLB
> > +	 * invalidation, 0 if none. Used to measure how late the ack for a
> > +	 * timed out invalidation actually arrives. Protected by
> > +	 * @pending_lock.
> > +	 */
> > +	int timedout_seqno;
> > +	/**
> > +	 * @timedout_inval_time: request time of @timedout_seqno. Protected by
> > +	 * @pending_lock.
> > +	 */
> > +	ktime_t timedout_inval_time;
> > +	/**
> > +	 * @timedout_time: time @timedout_seqno was signaled with -ETIME.
> > +	 * Protected by @pending_lock.
> > +	 */
> > +	ktime_t timedout_time;
> >  	/**
> >  	 * @fence_tdr: schedules a delayed call to xe_tlb_fence_timeout after
> >  	 * the timeout interval is over.
> > -- 
> > 2.55.0
> > 

  reply	other threads:[~2026-09-21  5:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 16:35 [PATCH v4 0/3] drm/xe: fix GuC TLB invalidation ack stalls on ARL (Wa_22016122933) Tales A. Mendonça
2026-09-17 16:35 ` [PATCH v4 1/3] drm/xe: Capture devcoredump on TLB invalidation timeout Tales A. Mendonça
2026-09-17 16:50   ` sashiko-bot
2026-09-17 23:18     ` Tales A. Mendonça
2026-09-18 22:01   ` Matthew Brost
2026-09-21 18:06     ` Tales A. Mendonça
2026-09-17 16:35 ` [PATCH v4 2/3] drm/xe: Log when a timed out TLB invalidation ack finally arrives Tales A. Mendonça
2026-09-18 22:06   ` Matthew Brost
2026-09-21  5:30     ` Matthew Brost [this message]
2026-09-21 18:10       ` Tales A. Mendonça
2026-09-21 18:19         ` Matthew Brost
2026-09-17 16:35 ` [PATCH v4 3/3] drm/xe: Implement Wa_22016122933 Tales A. Mendonça
2026-09-17 16:42 ` ✗ CI.checkpatch: warning for drm/xe: fix GuC TLB invalidation ack stalls on ARL (Wa_22016122933) (rev3) Patchwork
2026-09-17 16:44 ` ✓ CI.KUnit: success " Patchwork
2026-09-17 17:54 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-17 18:45 ` ✓ Xe.CI.FULL: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arDA2pUYH7uoUpE2@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=julia.filipchuk@intel.com \
    --cc=navonjohnlukose@gmail.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=stuart.summers@intel.com \
    --cc=talesam@gmail.com \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.