Messages in this thread | | | Subject | Re: drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59) | From | Chris Wilson <> | Date | Mon, 16 Nov 2020 10:15:04 +0000 |
| |
Quoting Dan Carpenter (2020-11-16 10:08:38) > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4 > commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling > config: i386-randconfig-m021-20201115 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > smatch warnings: > drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59) > > vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c > > 35 int > 36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data, > 37 struct drm_file *file) > 38 { > 39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES; > 40 struct drm_i915_file_private *file_priv = file->driver_priv; > 41 struct i915_gem_context *ctx; > 42 unsigned long idx; > 43 long ret; > 44 > 45 /* ABI: return -EIO if already wedged */ > 46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt); > 47 if (ret) > 48 return ret; > 49 > 50 rcu_read_lock(); > 51 xa_for_each(&file_priv->context_xa, idx, ctx) { > 52 struct i915_gem_engines_iter it; > 53 struct intel_context *ce; > 54 > 55 if (!kref_get_unless_zero(&ctx->ref)) > 56 continue; > 57 rcu_read_unlock(); > 58 > 59 for_each_gem_engine(ce, > 60 i915_gem_context_lock_engines(ctx), > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > I don't understand why this takes the lock every iteration through the > loop
It doesn't.
static inline struct i915_gem_engines * i915_gem_context_lock_engines(struct i915_gem_context *ctx) __acquires(&ctx->engines_mutex) { mutex_lock(&ctx->engines_mutex); return i915_gem_context_engines(ctx); }
static inline void i915_gem_context_unlock_engines(struct i915_gem_context *ctx) __releases(&ctx->engines_mutex) { mutex_unlock(&ctx->engines_mutex); }
with the i915_gem_engines stored as a local in the iterator at the start of the for loop. -Chris
| |