lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <c2f150f4a73739b42c3f70002073c2a5de11ef0e.camel@intel.com> Date: Thu, 3 Mar 2022 10:22:49 +0000 From: "Hellstrom, Thomas" <thomas.hellstrom@...el.com> To: "Souza, Jose" <jose.souza@...el.com>, "airlied@...ux.ie" <airlied@...ux.ie>, "jiasheng@...as.ac.cn" <jiasheng@...as.ac.cn>, "Vivi, Rodrigo" <rodrigo.vivi@...el.com>, "Sripada, Radhakrishna" <radhakrishna.sripada@...el.com>, "joonas.lahtinen@...ux.intel.com" <joonas.lahtinen@...ux.intel.com>, "ville.syrjala@...ux.intel.com" <ville.syrjala@...ux.intel.com>, "Roper, Matthew D" <matthew.d.roper@...el.com>, "tvrtko.ursulin@...ux.intel.com" <tvrtko.ursulin@...ux.intel.com>, "Auld, Matthew" <matthew.auld@...el.com>, "daniel@...ll.ch" <daniel@...ll.ch>, "jani.nikula@...ux.intel.com" <jani.nikula@...ux.intel.com>, "De Marchi, Lucas" <lucas.demarchi@...el.com> CC: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>, "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] drm/i915/gt: Handle errors for i915_gem_object_trylock 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@...as.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.
Powered by blists - more mailing lists