linux-kernel - Re: [PATCH] perf/core: Replace 1 with true

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>] [day] [month] [year] [list]

Date:   Fri, 11 Dec 2020 11:07:18 +0800
From:   Yejune Deng <yejune.deng@...il.com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     Peter Zijlstra <peterz@...radead.org>, mingo@...hat.com,
        acme@...nel.org, alexander.shishkin@...ux.intel.com,
        jolsa@...hat.com, namhyung@...nel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf/core: Replace 1 with true

Can my submission be accepted ? Do I need to do something, or wait for news?

On Wed, Dec 9, 2020 at 7:11 PM Mark Rutland <mark.rutland@....com> wrote:
>
> On Wed, Dec 09, 2020 at 11:37:19AM +0100, Peter Zijlstra wrote:
> > On Wed, Dec 09, 2020 at 06:09:01PM +0800, Yejune Deng wrote:
> > > In task_css_check(), the third parameter 'true' looks more readable.
> > >
> > > Signed-off-by: Yejune Deng <yejune.deng@...il.com>
> > > ---
> > >  kernel/events/core.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > > index d2f3ca7..0700022 100644
> > > --- a/kernel/events/core.c
> > > +++ b/kernel/events/core.c
> > > @@ -7265,7 +7265,7 @@ void perf_prepare_sample(struct perf_event_header *header,
> > >             struct cgroup *cgrp;
> > >
> > >             /* protected by RCU */
> > > -           cgrp = task_css_check(current, perf_event_cgrp_id, 1)->cgroup;
> > > +           cgrp = task_css_check(current, perf_event_cgrp_id, true)->cgroup;
> >
> > it looks like 3 wasted bytes to me
>
> I disagree; I think it'd make sense to do this for consistency, given all the
> other callers pass true/false (including in the header where this is defined):
>
> | [mark@...rids:~/src/linux]% git grep task_css_check
> | include/linux/cgroup.h: * task_css_check - obtain css for (task, subsys) w/ extra access conds
> | include/linux/cgroup.h:#define task_css_check(task, subsys_id, __c)                             \
> | include/linux/cgroup.h: * See task_css_check().
> | include/linux/cgroup.h: return task_css_check(task, subsys_id, false);
> | include/linux/cgroup.h: return task_css_check(task, subsys_id, true) ==
> | include/linux/perf_event.h:     return container_of(task_css_check(task, perf_event_cgrp_id,
> | kernel/cgroup/pids.c: * task_css_check(true) in pids_can_fork() and pids_cancel_fork() relies
> | kernel/cgroup/pids.c:           css = task_css_check(current, pids_cgrp_id, true);
> | kernel/cgroup/pids.c:           css = task_css_check(current, pids_cgrp_id, true);
> | kernel/events/core.c:           cgrp = task_css_check(current, perf_event_cgrp_id, 1)->cgroup;
> | kernel/sched/core.c:     * which is pointless here. Thus, we pass "true" to task_css_check()
> | kernel/sched/core.c:    tg = container_of(task_css_check(tsk, cpu_cgrp_id, true),
> | net/core/netclassid_cgroup.c:   return css_cls_state(task_css_check(p, net_cls_cgrp_id,
>
> ... but it'd be helpful if this could be cc'd to LKML
> (linux-kernel@...r.kernel.org), rather than just the maintainers off-list.
>
> ... or we could waste more than 3 bytes arguing. ;)
>
> Thanks,
> Mark.