lkml.org 
[lkml]   [2023]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] sched/psi: iterate through cgroups directly
On Wed, Feb 08, 2023 at 06:29:56PM +0100, Michal Koutný wrote:
> On Thu, Feb 09, 2023 at 12:16:54AM +0800, Kairui Song <ryncsn@gmail.com> wrote:
> > Signed-off-by: Kairui Song <kasong@tencent.com>
> > Signed-off-by: Kairui Song <ryncsn@gmail.com>
>
> Typo?
>
> > -static inline struct psi_group *task_psi_group(struct task_struct *task)
> > +static inline struct psi_group *psi_iter_first(struct task_struct *task, void **iter)
> > {
> > #ifdef CONFIG_CGROUPS
> > - if (static_branch_likely(&psi_cgroups_enabled))
> > - return cgroup_psi(task_dfl_cgroup(task));
> > + if (static_branch_likely(&psi_cgroups_enabled)) {
> > + struct cgroup *cgroup = task_dfl_cgroup(task);
> > +
> > + *iter = cgroup_parent(cgroup);
>
> This seems to skip a cgroup level -- maybe that's the observed
> performance gain?

Hm, I don't think it does. It sets up *iter to point to the parent for
the _next() call, but it returns task_dfl_cgroup()->psi. The next call
does the same: cgroup = *iter, *iter = parent, return cgroup->psi.

It could be a bit more readable to have *iter always point to the
current cgroup - but no strong preference either way from me:

psi_groups_first(task, iter)
{
#ifdef CONFIG_CGROUPS
if (static_branch_likely(&psi_cgroups_enabled)) {
struct cgroup *cgroup = task_dfl_cgroup(task);

*iter = cgroup;
return cgroup_psi(cgroup);
}
#endif
return &psi_system;
}

psi_groups_next(iter)
{
#ifdef CONFIG_CGROUPS
if (static_branch_likely(&psi_cgroups_enabled)) {
struct cgroup *cgroup = *iter;

if (cgroup) {
*iter = cgroup_parent(cgroup);
return cgroup_psi(cgroup);
}
}
return NULL;
#endif
}

\
 
 \ /
  Last update: 2023-03-27 00:15    [W:0.121 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site