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

[Luo Gengkun]

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@xxxxxxxxxxxxxxx>
> > ---
> >   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