dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Maarten Lankhorst" <dev@lankhorst.se>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4 4/7] drm/i915/gt: Fix selftests on PREEMPT_RT
Date: Mon, 06 Jul 2026 11:56:46 +0000	[thread overview]
Message-ID: <20260706115647.2CB801F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260706114823.752313-5-dev@lankhorst.se>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete implementation of PREEMPT_RT fixes for i915 selftests, leaving preemption-disabled busy-waits and delays that cause deadlocks and latency spikes.
--

commit c008dff1b2cefd9ee50ca924f83ff7c237a6fb3a
Author: Maarten Lankhorst <dev@lankhorst.se>

drm/i915/gt: Fix selftests on PREEMPT_RT

This commit modifies the i915 engine PM selftest to use migrate_disable()
instead of preempt_disable() around delay functions. This helps prevent 
latency issues on PREEMPT_RT kernels.

> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
> index 10e556a7eac45..c1eff9edd8a5e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
[ ... ]
> @@ -316,11 +316,11 @@ static int live_engine_busy_stats(void *arg)
>  		}
>  
>  		ENGINE_TRACE(engine, "measuring busy time\n");
> -		preempt_disable();
> +		migrate_disable();
>  		de = intel_engine_get_busy_time(engine, &t[0]);
>  		mdelay(100);
>  		de = ktime_sub(intel_engine_get_busy_time(engine, &t[1]), de);
> -		preempt_enable();
> +		migrate_enable();

[Severity: High]
This isn't a bug introduced by this patch, but since this series addresses
PREEMPT_RT issues in i915 selftests, are there similar preemption-disabled
delays and waits remaining in other selftests that should be addressed?

For example, looking at live_rps_clock_interval() in
drivers/gpu/drm/i915/gt/selftest_rps.c, there is a block that disables
preemption for at least 1 millisecond:

    preempt_disable();
    cycles_[i] = -intel_uncore_read_fw(gt->uncore, GEN6_RP_CUR_UP_EI);
    dt_[i] = ktime_get();
    
    udelay(1000);
    
    cycles_[i] += intel_uncore_read_fw(gt->uncore, GEN6_RP_CUR_UP_EI);
    dt_[i] = ktime_sub(ktime_get(), dt_[i]);
    preempt_enable();

Could this cause a severe latency spike on PREEMPT_RT?

Additionally, in drivers/gpu/drm/i915/selftests/i915_request.c,
measure_completion() disables preemption while busy-waiting for a callback:

    preempt_disable();
    semaphore_set(sema, i);
    while (!READ_ONCE(cb.seen))
        cpu_relax();
        
    elapsed[i - 1] = ENGINE_READ_FW(ce->engine, RING_TIMESTAMP);
    preempt_enable();

If the threaded handler that executes this callback is scheduled on the same
CPU on a PREEMPT_RT kernel, would this create a hard deadlock since the
busy-wait loop will never yield to the handler?

>  		dt = ktime_sub(t[1], t[0]);
>  		if (100 * de < 95 * dt || 95 * de > 100 * dt) {
>  			pr_err("%s: reported %lldns [%d%%] busyness while spinning [for %lldns]\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706114823.752313-1-dev@lankhorst.se?part=4

  reply	other threads:[~2026-07-06 11:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06 11:48 [PATCH v4 0/7] drm/i915: All fixes to make i915 work well with PREEMPT_RT Maarten Lankhorst
2026-07-06 11:48 ` [PATCH v4 1/7] drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock() Maarten Lankhorst
2026-07-06 12:03   ` sashiko-bot
2026-07-06 11:48 ` [PATCH v4 2/7] drm/i915: Drop the irqs_disabled() check Maarten Lankhorst
2026-07-06 11:48 ` [PATCH v4 3/7] drm/i915/guc: Consider also RCU depth in busy loop Maarten Lankhorst
2026-07-06 12:02   ` sashiko-bot
2026-07-06 11:48 ` [PATCH v4 4/7] drm/i915/gt: Fix selftests on PREEMPT_RT Maarten Lankhorst
2026-07-06 11:56   ` sashiko-bot [this message]
2026-07-06 11:48 ` [PATCH v4 5/7] drm/i915/gt: Set stop_timeout() correctly on PREEMPT-RT Maarten Lankhorst
2026-07-06 11:48 ` [PATCH v4 6/7] drm/i915: Use sleeping selftests for igt_atomic on PREEMPT_RT Maarten Lankhorst
2026-07-06 11:48 ` [PATCH v4 7/7] drm/i915/gt: Use signalers_lock to prevent starvation of irq_work Maarten Lankhorst
2026-07-06 12:04   ` sashiko-bot
2026-07-06 12:20 ` [PATCH v4 0/7] drm/i915: All fixes to make i915 work well with PREEMPT_RT Sebastian Andrzej Siewior
2026-07-06 12:51   ` Maarten Lankhorst

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=20260706115647.2CB801F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dev@lankhorst.se \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox