Quoting Lionel Landwerlin (2019-07-01 12:34:29)
> @@ -2535,9 +2635,21 @@ static int i915_perf_release(struct inode *inode, struct file *file)
> {
> struct i915_perf_stream *stream = file->private_data;
> struct drm_i915_private *dev_priv = stream->dev_priv;
> + struct i915_oa_config *oa_config, *next;
>
> mutex_lock(&dev_priv->perf.lock);
> +
> i915_perf_destroy_locked(stream);
> +
> + /* Dispose of all oa config batch buffers. */
> + mutex_lock(&dev_priv->perf.metrics_lock);
> + list_for_each_entry_safe(oa_config, next, &dev_priv->perf.metrics_buffers, vma_link) {
> + list_del(&oa_config->vma_link);
> + i915_gem_object_put(oa_config->obj);
> + oa_config->obj = NULL;
> + }
> + mutex_unlock(&dev_priv->perf.metrics_lock);
What's the reference chain from the i915_perf fd to the i915_device?
What's even keeping the module alive!
Shouldn't be a drm_dev_get() in i915_perf_open_ioctl() and a
drm_dev_put() here?
So there may be more than one stream, sharing the same oa_config. If a
stream closes, you let all the current streams keep their reference and
the next gets a new object. Looks like there's some scope for
duplication, but looks safe enough. My main worry was for zombie
oa_config.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
