Re: [PATCH linux-next 2/2] perf: Return EACCESS when need perfmon capability

[Luo Gengkun <luogengkun@huaweicloud.com>]

On 2025/1/6 23:59, James Clark wrote:
>
>
> On 23/12/2024 7:06 am, Luo Gengkun wrote:
>> For perf_allow_kernel and perf_allow_cpu, both return EACCES when 
>> require
>> CAP_PERFMON or CAP_SYS_ADMIN permissions, so update 
>> perf_allow_tracepoint
>> to keep them the same.
>>
>> Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
>> ---
>>   include/linux/perf_event.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>> index 5d2ec4283ebf..c1bc0d7a275b 100644
>> --- a/include/linux/perf_event.h
>> +++ b/include/linux/perf_event.h
>> @@ -1685,7 +1685,7 @@ static inline int perf_allow_cpu(void)
>>   static inline int perf_allow_tracepoint(void)
>>   {
>>       if (sysctl_perf_event_paranoid > -1 && !perfmon_capable())
>> -        return -EPERM;
>> +        return -EACCES;
>
> Is this necessary other than for consistency? If not it might be best 
> to leave it inconsistent even if it's wrong. I see quite a few "if 
> EPERM do this..." type things in Perf, so changing this would break 
> error messages being shown to users.
>
> If anything, EPERM seems more correct because EACCESS is more about 
> file access.
I think so, from the perspective of capabilities and 
sysctl_perf_event_paranoid, EPERM is more appropriate.
>
> Thanks
> James


Thanks for your review.

Actually, I am not sure if it's typo or intentional, so this patch is 
more like a consultation. It's okay to keep it the same so it doesn't 
torture the user.


Thanks

Gengkun