On Mon, May 06, 2024 at 05:29:31AM +0000, Mingwei Zhang wrote:
> +int perf_get_mediated_pmu(void)
> +{
> + int ret = 0;
> +
> + mutex_lock(&perf_mediated_pmu_mutex);
> + if (refcount_inc_not_zero(&nr_mediated_pmu_vms))
> + goto end;
> +
> + if (atomic_read(&nr_include_guest_events)) {
> + ret = -EBUSY;
> + goto end;
> + }
> + refcount_set(&nr_mediated_pmu_vms, 1);
> +end:
> + mutex_unlock(&perf_mediated_pmu_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(perf_get_mediated_pmu);
> +
> +void perf_put_mediated_pmu(void)
> +{
> + if (!refcount_dec_not_one(&nr_mediated_pmu_vms))
> + refcount_set(&nr_mediated_pmu_vms, 0);
I'm sorry, but this made the WTF'o'meter go 'ding'.
Isn't that simply refcount_dec() ?
> +}
> +EXPORT_SYMBOL_GPL(perf_put_mediated_pmu);
> +
> /*
> * Holding the top-level event's child_mutex means that any
> * descendant process that has inherited this event will block
> @@ -12086,11 +12140,24 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
> if (err)
> goto err_callchain_buffer;
>
> + if (is_include_guest_event(event)) {
> + mutex_lock(&perf_mediated_pmu_mutex);
> + if (refcount_read(&nr_mediated_pmu_vms)) {
> + mutex_unlock(&perf_mediated_pmu_mutex);
> + err = -EACCES;
> + goto err_security_alloc;
> + }
> + atomic_inc(&nr_include_guest_events);
> + mutex_unlock(&perf_mediated_pmu_mutex);
> + }
Wouldn't all that be nicer with a helper function?
if (is_include_guest_event() && !perf_get_guest_event())
goto err_security_alloc;
> +
> /* symmetric to unaccount_event() in _free_event() */
> account_event(event);
>
> return event;
>
> +err_security_alloc:
> + security_perf_event_free(event);
> err_callchain_buffer:
> if (!event->parent) {
> if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
