Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1607508541-9345-1-git-send-email-yejune.deng@gmail.com>
 <20201209103719.GV3021@hirez.programming.kicks-ass.net> <20201209110405.GA5601@C02TD0UTHF1T.local>
In-Reply-To: <20201209110405.GA5601@C02TD0UTHF1T.local>
From:   Yejune Deng <yejune.deng@gmail.com>
Date:   Fri, 11 Dec 2020 11:07:18 +0800
Message-ID: <CABWKuGWHpMkfbQQ5v_6y=vnO0joMJhaEvZvnd3AjUtLzuXiXaA@mail.gmail.com>
Subject: Re: [PATCH] perf/core: Replace 1 with true
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     Peter Zijlstra <peterz@infradead.org>, mingo@redhat.com,
        acme@kernel.org, alexander.shishkin@linux.intel.com,
        jolsa@redhat.com, namhyung@kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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@arm.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@gmail.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@lakrids:~/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@vger.kernel.org), rather than just the maintainers off-list.
>
> ... or we could waste more than 3 bytes arguing. ;)
>
> Thanks,
> Mark.