[Intel-gfx] [PATCH v6 03/11] drm/i915/perf: allow for CS OA configs to be created lazily

Chris Wilson chris at chris-wilson.co.uk
Mon Jul 1 13:06:48 UTC 2019


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


More information about the Intel-gfx mailing list