From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 87219CA5FFD for ; Mon, 5 Oct 2026 09:39:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 328F210EC46; Mon, 5 Oct 2026 09:39:32 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=collabora.com header.i=@collabora.com header.b="eOmETh0u"; dkim-atps=neutral Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2735110E1E8; Mon, 5 Oct 2026 09:39:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1791193168; bh=O6YBEBOgxU2xPbzAN0XEomFzmcuWs7cjwFLomcSls14=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=eOmETh0ucLVcoEAOdMbhTfYydfCSTpA69TNPABDhvieR8E7Wm1iMZe1f3mDefxmsY Ao+GvEIAXv3HwwwmbA8fKhFC/WWlTwq4GBTNTIbI4fPAzcWQKRl6FnXyyAkfIKrBNl syv9jHwAEw0zfNM6kqfSDODm/gdXLV9ZvFJX/LLZQyh20vGyZiAaccT9ZAyEX2P/nl RYhL4nBdmyaEMB8Gtqlr+g3KfAtOYRoA+RpM7OmSbgWtb+UbfIUkqAz2yVcIAank19 y6iFyldyp+H0IGtl6Cd8yaxv/cs8zwp+Zej9gvNlO7e5vnk8kZ4uQOfm3cWFf7ajs4 QPlMUjELoNfEA== Received: from fedora-61.home (unknown [100.64.0.11]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange secp256r1 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id B374D17E01C2; Mon, 05 Oct 2026 11:39:27 +0200 (CEST) Date: Mon, 5 Oct 2026 11:39:24 +0200 From: Boris Brezillon To: Matthew Brost Cc: intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, Abhinav Kumar , Alice Ryhl , Anna Maniscalco , Antonino Maniscalco , Danilo Krummrich , David Airlie , Dmitry Baryshkov , Jessica Zhang , Jonathan Corbet , Liviu Dudau , Lyude Paul , Maarten Lankhorst , Marijn Suijten , Maxime Ripard , Randy Dunlap , Rob Clark , Rodrigo Vivi , Sean Paul , Shuah Khan , Simona Vetter , Steven Price , Thomas =?UTF-8?B?SGVsbHN0csO2bQ==?= , Thomas Zimmermann Subject: Re: [PATCH v3 3/8] drm/panthor: lock the resident BOs of a submit last Message-ID: <20261005113924.37276be0@fedora-61.home> In-Reply-To: <20261001220632.3190896-4-matthew.brost@intel.com> References: <20261001220632.3190896-1-matthew.brost@intel.com> <20261001220632.3190896-4-matthew.brost@intel.com> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 1 Oct 2026 15:06:27 -0700 Matthew Brost wrote: > panthor_vm_prepare_mapped_bos_resvs() locks every external object mapped > in the VM and then validates the evicted ones. Validation here means > panthor_vm_bo_validate(), which swaps the BO's pages back in and restores > its VMAs. That is slow, and an external object is one which can be shared > with another process, so the whole of it happens while holding dma-resv > locks other processes may be waiting on. >=20 > Nothing is gained by holding those. A resident object needs no swapping > in; only the evicted ones do. Split the locking into the two passes > gpuvm now understands: the early pass takes just the evicted external > objects and swaps them in, and the late pass takes the ones which were > resident and are therefore normally ready to use as they are. Private > objects are covered by the VM resv, which is held from the start, so > evicted ones are still validated in the early pass. >=20 > The split is only worth it when there is something to validate, so > drm_gpuvm_exec_pass_needs_split() decides, and a submit with nothing > evicted keeps doing exactly what it does today in a single pass. >=20 > Both passes run in the same drm_exec transaction, so nothing is unlocked > in between and the late pass only ever adds locks. They take disjoint > sets of objects, so passing slot_count to both still reserves it exactly > once per object. >=20 > The early pass reads the evicted state without the object's dma-resv, > that being the lock it is trying not to take. The race is benign: an > object evicted right after the early pass skipped it is picked up by the > late pass instead, which is why that pass still validates. >=20 > Validation here allocates pages, which can recurse into panthor's own > shrinker, so it is worth being explicit about what the early pass can > evict. There is no deadlock: drm_gem_lru_scan() acquires the resv with > ww_mutex_trylock() and skips what it cannot get. VM-exclusive BOs share > the VM resv, which is held across both passes, so those are always > skipped. External objects are not held by the early pass, though, so > reclaim can evict one while the early pass validates something else. >=20 > That is handled, and is why the late pass validates rather than only > locking: it picks up anything evicted after the early pass looked at it. > The cost is that the swapin for such a BO happens under the full set of > locks, i.e. it degrades to the current behaviour for that one object. >=20 > Xe avoids this by refusing to evict BOs bound to a VM the current task is > validating (xe_bo_eviction_valuable() and xe_vm_is_validating()). Panthor > has no equivalent guard. Adding one would make the split more effective > under memory pressure, but it is not needed for correctness, so it is left > as a follow up. >=20 > Cc: Abhinav Kumar > Cc: Alice Ryhl > Cc: Anna Maniscalco > Cc: Antonino Maniscalco > Cc: Boris Brezillon Reviewed-by: Boris Brezillon > Cc: Danilo Krummrich > Cc: David Airlie > Cc: Dmitry Baryshkov > Cc: Jessica Zhang > Cc: Jonathan Corbet > Cc: Liviu Dudau > Cc: Lyude Paul > Cc: Maarten Lankhorst > Cc: Marijn Suijten > Cc: Maxime Ripard > Cc: Randy Dunlap > Cc: Rob Clark > Cc: Rodrigo Vivi > Cc: Sean Paul > Cc: Shuah Khan > Cc: Simona Vetter > Cc: Steven Price > Cc: Thomas Hellstr=C3=B6m > Cc: Thomas Zimmermann > Signed-off-by: Matthew Brost > Assisted-by: LLM > Reviewed-by: Liviu Dudau > --- > v3: > - Follow the drm_gpuvm_exec_pass_ function renames (Danilo) > --- > drivers/gpu/drm/panthor/panthor_mmu.c | 55 ++++++++++++++++++++++++++- > 1 file changed, 53 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/pant= hor/panthor_mmu.c > index d75d575473da..d4b968ab094e 100644 > --- a/drivers/gpu/drm/panthor/panthor_mmu.c > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c > @@ -3258,6 +3258,26 @@ int panthor_vm_unmap_range(struct panthor_vm *vm, = u64 va, u64 size) > * need to reserve a slot on all BOs mapped to a VM and update this slot= with > * the job fence after its submission. > * > + * When something is evicted the locks are taken in two passes; when not= hing > + * is, a single pass is used, as before. The early pass only takes the e= xternal > + * objects which actually need validating, i.e. the evicted ones, and sw= aps > + * them back in. Private objects are covered by the VM resv, which is he= ld > + * from the start, so they are validated here too. The late pass then ta= kes > + * the external objects the early pass left out, which were resident and= so > + * normally need no swapping in; it still validates, since one of them m= ay > + * have been evicted in the meantime. > + * > + * The point is that panthor_vm_bo_validate() swaps pages back in, which= is > + * slow, and an external object is one which can be shared with another > + * process. Doing that while holding the resv of a resident shared BO wo= uld > + * stall whoever else needs it, for no benefit, since a resident object = is > + * ready to use as it is. > + * > + * Both passes run in the same drm_exec transaction: nothing is unlocked= in > + * between and the late pass only ever adds locks. The passes take disjo= int > + * sets of objects, so reserving @slot_count in each still reserves it > + * exactly once per object. > + * > * Return: 0 on success, a negative error code otherwise. > */ > int panthor_vm_prepare_mapped_bos_resvs(struct drm_exec *exec, struct pa= nthor_vm *vm, > @@ -3270,11 +3290,42 @@ int panthor_vm_prepare_mapped_bos_resvs(struct dr= m_exec *exec, struct panthor_vm > if (ret) > return ret; > =20 > - ret =3D drm_gpuvm_prepare_objects(&vm->base, exec, slot_count); > + /* > + * With nothing evicted there is no validation to keep the resident > + * objects unlocked for, so do not pay for the second walk. > + */ > + if (!drm_gpuvm_exec_pass_needs_split(&vm->base)) { > + ret =3D drm_gpuvm_prepare_objects(&vm->base, exec, slot_count); > + if (ret) > + return ret; > + > + return drm_gpuvm_validate(&vm->base, exec); > + } > + > + ret =3D drm_gpuvm_exec_pass_prepare_objects(&vm->base, exec, > + slot_count, > + DRM_GPUVM_EXEC_PASS_EARLY); > + if (ret) > + return ret; > + > + ret =3D drm_gpuvm_exec_pass_validate(&vm->base, exec, > + DRM_GPUVM_EXEC_PASS_EARLY); > if (ret) > return ret; > =20 > - return drm_gpuvm_validate(&vm->base, exec); > + ret =3D drm_gpuvm_exec_pass_prepare_objects(&vm->base, exec, > + slot_count, > + DRM_GPUVM_EXEC_PASS_LATE); > + if (ret) > + return ret; > + > + /* > + * Objects the early pass skipped were resident then, but another > + * process may have evicted one since. Now that everything is locked, > + * pick up whatever is left. > + */ > + return drm_gpuvm_exec_pass_validate(&vm->base, exec, > + DRM_GPUVM_EXEC_PASS_LATE); > } > =20 > unsigned long