All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Natalie Vock" <nat@pixelcluster.dev>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 09/10] drm/ttm: support using drm_exec during eviction v4
Date: Mon, 06 Jul 2026 10:36:29 +0000	[thread overview]
Message-ID: <20260706103629.D89451F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260706-ttm_2_drm_exec-v2-9-4bf6bfc0d320@pixelcluster.dev>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The logic change silently overrides `trylock_only` and `no_wait_gpu` flags when `ctx->exec` is provided, forcing blocking locks instead of trylocks.
--

commit 5fb376340fb1686ce3b629a364137351ecc0b343
Author: Natalie Vock <nat@pixelcluster.dev>

drm/ttm: support using drm_exec during eviction v4

Allow specifying a drm_exec object in TTMs operation context which is
used to lock objects during eviction.

> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index a53b25e8c2967..96699532817c2 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
[ ... ]
> @@ -837,6 +838,8 @@ static bool ttm_lru_walk_trylock(struct ttm_bo_lru_cursor *curs,
>  	struct ttm_operation_ctx *ctx = curs->arg->ctx;
>  
>  	curs->needs_unlock = false;
> +	if (ctx->exec)
> +		return false;

[Severity: High]
Does returning false here when ctx->exec is set force the walk to bypass
trylocking and push the logic into a blocking fallback?

>  
>  	if (dma_resv_trylock(bo->base.resv)) {
>  		curs->needs_unlock = true;
[ ... ]
> @@ -857,7 +860,9 @@ static int ttm_lru_walk_ticketlock(struct ttm_bo_lru_cursor *curs,
>  	struct ttm_lru_walk_arg *arg = curs->arg;
>  	int ret;
>  
> -	if (arg->ctx->interruptible)
> +	if (arg->ctx->exec)
> +		ret = drm_exec_lock_obj_report_dup(arg->ctx->exec, &bo->base);

[Severity: High]
Does this perform a blocking WW mutex lock via drm_exec, completely ignoring
the trylock_only intent of the caller?

> +	else if (arg->ctx->interruptible)
>  		ret = dma_resv_lock_interruptible(bo->base.resv, arg->ticket);
>  	else
>  		ret = dma_resv_lock(bo->base.resv, arg->ticket);
[ ... ]
> @@ -1016,8 +1030,8 @@ __ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
>  		if (ttm_lru_walk_trylock(curs, bo)) {
>  			bo_locked = true;
>  
> -		} else if (!arg->ticket || arg->ctx->no_wait_gpu ||
> -			   arg->trylock_only) {
> +		} else if ((!arg->ticket || arg->ctx->no_wait_gpu ||
> +			    arg->trylock_only) && !arg->ctx->exec) {

[Severity: High]
Does negating the non-blocking flags when ctx->exec is present force the
walk to fall through to a blocking lock attempt?

This appears to silently override the trylock_only and no_wait_gpu flags.
When amdgpu_cs_bo_validate() triggers TTM eviction under memory pressure, it
passes an operation context with ctx->exec set. ttm_bo_evict_first() initiates
its first pass with trylock_only = true to find an idle buffer.

Because of these changes, the walk bypasses the trylock_only check and falls
through to ttm_lru_walk_ticketlock(), which calls
drm_exec_lock_obj_report_dup() and performs a blocking WW mutex lock.

Could this cause the eviction to block and wait on the first busy buffer in
the LRU instead of skipping it, potentially breaking the two-pass eviction
strategy and leading to unnecessary command submission backoffs or
serialization of independent workloads?

>  			spin_unlock(lru_lock);
>  			ttm_bo_put(bo);
>  			spin_lock(lru_lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-ttm_2_drm_exec-v2-0-4bf6bfc0d320@pixelcluster.dev?part=9

  reply	other threads:[~2026-07-06 10:36 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06 10:07 [PATCH v2 00/10] Use drm_exec to lock TTM buffers, respin Natalie Vock
2026-07-06 10:07 ` [PATCH v2 01/10] drm/exec: Add helper to bypass IGNORE_DUPLICATES flag Natalie Vock
2026-07-06 11:40   ` Christian König
2026-07-06 10:07 ` [PATCH v2 02/10] drm/ttm: replace TTMs refcount with the DRM refcount v4 Natalie Vock
2026-07-06 10:42   ` sashiko-bot
2026-07-06 11:47   ` Christian König
2026-07-06 10:07 ` [PATCH v2 03/10] drm/ttm: remove ttm_lru_walk_ops Natalie Vock
2026-07-06 10:07 ` [PATCH v2 04/10] drm/ttm: grab BO reference before locking it Natalie Vock
2026-07-06 10:07 ` [PATCH v2 05/10] drm/ttm: switch to ttm_bo_lru_for_each_reserved_guarded for swapout Natalie Vock
2026-07-06 10:32   ` sashiko-bot
2026-07-06 10:07 ` [PATCH v2 06/10] drm/ttm: move zombie handling into ttm_bo_evict Natalie Vock
2026-07-06 10:07 ` [PATCH v2 07/10] drm/ttm: use ttm_bo_lru_for_each_reserved_guarded in evict_all Natalie Vock
2026-07-06 10:34   ` sashiko-bot
2026-07-06 10:07 ` [PATCH v2 08/10] drm/xe: remove workaround for TTM internals Natalie Vock
2026-07-06 10:32   ` sashiko-bot
2026-07-06 10:07 ` [PATCH v2 09/10] drm/ttm: support using drm_exec during eviction v4 Natalie Vock
2026-07-06 10:36   ` sashiko-bot [this message]
2026-07-06 10:07 ` [PATCH v2 10/10] drm/amdgpu: use drm_exec during BO validation Natalie Vock
2026-07-06 10:41   ` sashiko-bot
2026-07-06 12:40 ` ✗ LGCI.VerificationFailed: failure for Use drm_exec to lock TTM buffers, respin (rev2) Patchwork
2026-07-06 13:02 ` Patchwork
2026-07-06 14:41 ` ✗ CI.checkpatch: warning for Use drm_exec to lock TTM buffers, respin (rev3) Patchwork
2026-07-06 14:42 ` ✗ CI.KUnit: failure " Patchwork
2026-07-06 23:32 ` ✗ i915.CI.BAT: " 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=20260706103629.D89451F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=nat@pixelcluster.dev \
    --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 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.