Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] drm/xe: data race in xe_tlb_inval_fence_timeout() reading seqno_recv --
commit ef794f6bbaafa679257b9fec9405245af4942e68 Author: Tales A. Mendonça <[email protected]> drm/xe: Capture devcoredump on TLB invalidation timeout This commit adds xe_devcoredump_gt(), a variant of xe_devcoredump() for hangs that are not tied to an exec queue or job. It hooks this new capture mechanism into the TLB invalidation timeout path to capture the GuC log and CT state. Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678 > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c > b/drivers/gpu/drm/xe/xe_tlb_inval.c > index 7c3bf32d9258b..7a0c04fac60aa 100644 > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c [ ... ] > @@ -97,6 +107,16 @@ static void xe_tlb_inval_fence_timeout(struct work_struct > *work) > queue_delayed_work(tlb_inval->timeout_wq, &tlb_inval->fence_tdr, > timeout_delay); > spin_unlock_irq(&tlb_inval->pending_lock); > + > + /* > + * Capture the GuC log and CT state so the firmware side of the hang > + * can be inspected; there is no queue or job to blame here. Must be > + * outside pending_lock as the capture takes sleeping locks. > + */ > + if (timedout_seqno) > + xe_devcoredump_gt(tlb_inval_to_gt(tlb_inval), > + "TLB invalidation fence timeout, seqno=%d > recv=%d", > + timedout_seqno, tlb_inval->seqno_recv); [Severity: Low] Does this code introduce a data race when reading tlb_inval->seqno_recv? In xe_tlb_inval_fence_timeout() in drivers/gpu/drm/xe/xe_tlb_inval.c, the pending_lock spinlock is dropped just before this block. Because seqno_recv is read locklessly here, and concurrent updates (such as those in xe_tlb_inval_done_handler()) use WRITE_ONCE(), reading it directly could result in a data race. Could this lead to KCSAN warnings or logging a corrupted value in the devcoredump reason string? Would it be safer to read this using READ_ONCE() or cache the value while the lock is held? > } > > /** -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
