On Sun, Sep 20, 2026 at 10:30:00PM -0700, Matthew Brost wrote: > 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 <[email protected]>
Thanks Matt, and thanks for taking the question to the people who own SIGID rather than letting it sit. I will carry your tag on this patch in v5. Agreed on leaving the SIGID conversion as a follow-up: there is no TLB component in DEFINE_XE_LOG_COMPONENTS() yet, so adding one touches the log ABI and belongs with the people doing that work rather than bolted onto a bug fix. Happy to rebase on top of it once it lands. v5 is otherwise ready and goes out this week. The only code delta is in patch 1, from the data race Sashiko spotted after v4. While you are here - patch 3 is the one that actually stops the stalls, and it is still the only one without review. It applies XE_BO_FLAG_NEEDS_UC to the GuC-shared allocations (CTBs, log, ADS, SLPC, engine activity) on the standalone media GT, scoped by a new OOB rule (22016122933 MEDIA_VERSION(1300)). The approach was Daniele's suggestion. It has been running on two ARL machines for about six weeks across several kernel versions, with over 10 million invalidations and zero stalls; before it, both machines hit the ~2.3s ack delay 20-60 times a day. Daniele, Stuart, would either of you be able to take a look at that one? Tales Em seg., 21 de set. de 2026 às 02:30, Matthew Brost <[email protected]> escreveu: > > 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 <[email protected]> > > > > 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 <[email protected]> > > > > --- > > > 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 > > > -- Com os cumprimentos, Tales A. Mendonça talesam.org communitybig.org
