Quoting Lionel Landwerlin (2019-07-01 12:34:29)
> struct i915_oa_config {
> + struct drm_i915_private *i915;
> +
> char uuid[UUID_STRING_LEN + 1];
> int id;
>
> @@ -1110,6 +1112,10 @@ struct i915_oa_config {
> struct attribute *attrs[2];
> struct device_attribute sysfs_metric_id;
>
> + struct drm_i915_gem_object *obj;
> +
> + struct list_head vma_link;
> +
> atomic_t ref_count;
> };
> -static void free_oa_config(struct drm_i915_private *dev_priv,
> - struct i915_oa_config *oa_config)
> +static void put_oa_config(struct i915_oa_config *oa_config)
> {
> + if (!atomic_dec_and_test(&oa_config->ref_count))
> + return;
I strongly advise that ref_count be replaced by struct kref, just so that
we get the benefit of debugging.
put_oa_config -> kref_put(&oa_config->ref, free_oa_config)
(free_oa_config takes kref as its arg and uses container_of())
> +int i915_perf_get_oa_config(struct drm_i915_private *i915,
> + int metrics_set,
> + struct i915_oa_config **out_config,
> + struct drm_i915_gem_object **out_obj)
> +{
> + int ret = 0;
> + struct i915_oa_config *oa_config;
> +
> + if (!i915->perf.initialized)
> + return -ENODEV;
> +
> + ret = mutex_lock_interruptible(&i915->perf.metrics_lock);
> if (ret)
> return ret;
>
> - *out_config = idr_find(&dev_priv->perf.metrics_idr, metrics_set);
> - if (!*out_config)
> - ret = -EINVAL;
> - else
> - atomic_inc(&(*out_config)->ref_count);
> + if (metrics_set == 1) {
> + oa_config = &i915->perf.oa.test_config;
> + } else {
> + oa_config = idr_find(&i915->perf.metrics_idr, metrics_set);
Why not have the builtin[1] inside the idr?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
