linux-kernel - Re: [PATCH 06/19] perf: Simplify perf_pmu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d64ebfba-49db-4b04-9a84-b9ecd26e6c76@amd.com>
Date: Wed, 20 Nov 2024 18:36:55 +0530
From: Ravi Bangoria <ravi.bangoria@....com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mingo@...nel.org, lucas.demarchi@...el.com, linux-kernel@...r.kernel.org,
 willy@...radead.org, acme@...nel.org, namhyung@...nel.org,
 mark.rutland@....com, alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
 irogers@...gle.com, adrian.hunter@...el.com, kan.liang@...ux.intel.com,
 Ravi Bangoria <ravi.bangoria@....com>
Subject: Re: [PATCH 06/19] perf: Simplify perf_pmu_register()

Hi Peter,

> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11778,52 +11778,49 @@ static void perf_pmu_free(struct pmu *pm
>  	free_percpu(pmu->cpu_pmu_context);
>  }
>  
> -int perf_pmu_register(struct pmu *pmu, const char *name, int type)
> +DEFINE_FREE(pmu_unregister, struct pmu *, if (_T) perf_pmu_free(_T))
> +
> +int perf_pmu_register(struct pmu *_pmu, const char *name, int type)
>  {
> -	int cpu, ret, max = PERF_TYPE_MAX;
> +	int cpu, max = PERF_TYPE_MAX;
>  
> -	pmu->type = -1;
> +	struct pmu *pmu __free(pmu_unregister) = _pmu;
> +	guard(mutex)(&pmus_lock);
>  
> -	mutex_lock(&pmus_lock);
> -	ret = -ENOMEM;
>  	pmu->pmu_disable_count = alloc_percpu(int);
>  	if (!pmu->pmu_disable_count)
> -		goto unlock;
> +		return -ENOMEM;
>  
> -	if (WARN_ONCE(!name, "Can not register anonymous pmu.\n")) {
> -		ret = -EINVAL;
> -		goto free;
> -	}
> +	if (WARN_ONCE(!name, "Can not register anonymous pmu.\n"))
> +		return -EINVAL;
>  
> -	if (WARN_ONCE(pmu->scope >= PERF_PMU_MAX_SCOPE, "Can not register a pmu with an invalid scope.\n")) {
> -		ret = -EINVAL;
> -		goto free;
> -	}
> +	if (WARN_ONCE(pmu->scope >= PERF_PMU_MAX_SCOPE,
> +		      "Can not register a pmu with an invalid scope.\n"))
> +		return -EINVAL;
>  
>  	pmu->name = name;
>  
>  	if (type >= 0)
>  		max = type;
>  
> -	ret = idr_alloc(&pmu_idr, NULL, max, 0, GFP_KERNEL);
> -	if (ret < 0)
> -		goto free;
> +	CLASS(idr_alloc, pmu_type)(&pmu_idr, NULL, max, 0, GFP_KERNEL);
> +	if (pmu_type.id < 0)
> +		return pmu_type.id;
>  
> -	WARN_ON(type >= 0 && ret != type);
> +	WARN_ON(type >= 0 && pmu_type.id != type);
>  
> -	pmu->type = ret;
> +	pmu->type = pmu_type.id;
>  	atomic_set(&pmu->exclusive_cnt, 0);
>  
>  	if (pmu_bus_running && !pmu->dev) {
> -		ret = pmu_dev_alloc(pmu);
> +		int ret = pmu_dev_alloc(pmu);
>  		if (ret)
> -			goto free;
> +			return ret;

pmu_dev_alloc() can fail before or in device_add(). perf_pmu_free() should
not call device_del() for such cases. No?

Thanks,
Ravi