On Wed, 2022-03-02 at 10:37 +0000, Tvrtko Ursulin wrote:

• Thomas, Matt

On 02/03/2022 06:19, Jiasheng Jiang wrote:

...

As the potential failure of the i915_gem_object_trylock(), it should be better to check it and return error if fails.

Fixes: 94ce0d65076c ("drm/i915/gt: Setup a default migration context on the GT") Signed-off-by: Jiasheng Jiang jiasheng@iscas.ac.cn

---

drivers/gpu/drm/i915/gt/selftest_migrate.c | 6 +++++-   1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_migrate.c b/drivers/gpu/drm/i915/gt/selftest_migrate.c index fa4293d2944f..79c6c68f7316 100644 --- a/drivers/gpu/drm/i915/gt/selftest_migrate.c +++ b/drivers/gpu/drm/i915/gt/selftest_migrate.c @@ -465,7 +465,11 @@ create_init_lmem_internal(struct intel_gt *gt, size_t sz, bool try_lmem)                         return obj;         }   -       i915_gem_object_trylock(obj, NULL);

Guys why is this a trylock to start with? (Since being added in 94ce0d65076c ("drm/i915/gt: Setup a default migration context on the GT").

Surely it can't ever fail since the object has just been created.

Typically in some situations, we want to create locked objects when we're already in a ww transaction and might not have access to the ww context, in which case an ordinary sleeping lock would trigger a lockdep splat, so a trylock is used instead since it will never fail.

I once introduced a i915_gem_object_lock_isolated() wrapper around trylock to annotate this situation, but that was removed during the obj->mm.lock removal IIRC. Similarly TTM has an option to create an object locked.

So here we should probably BUG on a trylock failure, if anything. In the long run we should probably mimic TTM and introduce an interface to create an object locked.

/Thomas

Regards,

Tvrtko

...

+       if (!i915_gem_object_trylock(obj, NULL)) { +               i915_gem_object_put(obj); +               return ERR_PTR(-EBUSY); +       }

err = i915_gem_object_pin_pages(obj);         if (err) {                 i915_gem_object_unlock(obj);

---------------------------------------------------------------------- Intel Sweden AB Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.