| Message ID | 20231222113102.4148-2-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Add bpf_iter_cpumask | expand |
Hello, On Fri, Dec 22, 2023 at 11:30:59AM +0000, Yafang Shao wrote: > By initializing the root cgroup's psi field to psi_system, we can > consistently obtain the psi information for all cgroups from the struct > cgroup. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > include/linux/psi.h | 2 +- > kernel/cgroup/cgroup.c | 5 ++++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/include/linux/psi.h b/include/linux/psi.h > index e074587..8f2db51 100644 > --- a/include/linux/psi.h > +++ b/include/linux/psi.h > @@ -34,7 +34,7 @@ __poll_t psi_trigger_poll(void **trigger_ptr, struct file *file, > #ifdef CONFIG_CGROUPS > static inline struct psi_group *cgroup_psi(struct cgroup *cgrp) > { > - return cgroup_ino(cgrp) == 1 ? &psi_system : cgrp->psi; > + return cgrp->psi; > } How have you tested this change? Looking at the code there are other references to psi_system, e.g. to show it under /proc/pressure/* and to exempt it from CPU FULL accounting. I don't see how the above change would be sufficient. Thanks.
Hi Yafang, kernel test robot noticed the following build errors: [auto build test ERROR on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-psi-Init-PSI-of-root-cgroup-to-psi_system/20231222-193221 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20231222113102.4148-2-laoar.shao%40gmail.com patch subject: [PATCH bpf-next 1/4] cgroup, psi: Init PSI of root cgroup to psi_system config: s390-randconfig-r081-20231223 (https://download.01.org/0day-ci/archive/20231223/202312230748.92S9ML64-lkp@intel.com/config) compiler: s390-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231223/202312230748.92S9ML64-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312230748.92S9ML64-lkp@intel.com/ All errors (new ones prefixed by >>): >> kernel/cgroup/cgroup.c:169:22: error: 'psi_system' undeclared here (not in a function) 169 | .cgrp.psi = &psi_system, | ^~~~~~~~~~ vim +/psi_system +169 kernel/cgroup/cgroup.c 165 166 /* the default hierarchy */ 167 struct cgroup_root cgrp_dfl_root = { 168 .cgrp.rstat_cpu = &cgrp_dfl_root_rstat_cpu, > 169 .cgrp.psi = &psi_system, 170 }; 171 EXPORT_SYMBOL_GPL(cgrp_dfl_root); 172
Hi Yafang, kernel test robot noticed the following build errors: [auto build test ERROR on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Yafang-Shao/cgroup-psi-Init-PSI-of-root-cgroup-to-psi_system/20231222-193221 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20231222113102.4148-2-laoar.shao%40gmail.com patch subject: [PATCH bpf-next 1/4] cgroup, psi: Init PSI of root cgroup to psi_system config: arm-defconfig (https://download.01.org/0day-ci/archive/20231223/202312231522.VWy0LXXY-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231223/202312231522.VWy0LXXY-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312231522.VWy0LXXY-lkp@intel.com/ All errors (new ones prefixed by >>): >> kernel/cgroup/cgroup.c:169:15: error: use of undeclared identifier 'psi_system' .cgrp.psi = &psi_system, ^ 1 error generated. vim +/psi_system +169 kernel/cgroup/cgroup.c 165 166 /* the default hierarchy */ 167 struct cgroup_root cgrp_dfl_root = { 168 .cgrp.rstat_cpu = &cgrp_dfl_root_rstat_cpu, > 169 .cgrp.psi = &psi_system, 170 }; 171 EXPORT_SYMBOL_GPL(cgrp_dfl_root); 172
On Sat, Dec 23, 2023 at 1:47 AM Tejun Heo <tj@kernel.org> wrote: > > Hello, > > On Fri, Dec 22, 2023 at 11:30:59AM +0000, Yafang Shao wrote: > > By initializing the root cgroup's psi field to psi_system, we can > > consistently obtain the psi information for all cgroups from the struct > > cgroup. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > include/linux/psi.h | 2 +- > > kernel/cgroup/cgroup.c | 5 ++++- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/psi.h b/include/linux/psi.h > > index e074587..8f2db51 100644 > > --- a/include/linux/psi.h > > +++ b/include/linux/psi.h > > @@ -34,7 +34,7 @@ __poll_t psi_trigger_poll(void **trigger_ptr, struct file *file, > > #ifdef CONFIG_CGROUPS > > static inline struct psi_group *cgroup_psi(struct cgroup *cgrp) > > { > > - return cgroup_ino(cgrp) == 1 ? &psi_system : cgrp->psi; > > + return cgrp->psi; > > } > > How have you tested this change? Looking at the code there are other After implementing the modification, I solely focused on validating the functionality of root_cgrp->psi to ensure its compatibility with the recent changes, akin to the self-tests performed in the previous version [0]. However, it's noteworthy that building the kernel necessitates clang-14+, hence, I refrained from incorporating this into the current version. Regarding the alterations made to /proc/pressure/, I haven't yet conducted thorough verification to confirm if the adjustments are comprehensive enough. I will analyze the potential impact on /proc/pressure/* in the next phase. [0]. https://lore.kernel.org/bpf/20230801142912.55078-4-laoar.shao@gmail.com/ > references to psi_system, e.g. to show it under /proc/pressure/* and to > exempt it from CPU FULL accounting. I don't see how the above change would > be sufficient. Thanks for your suggestion.
diff --git a/include/linux/psi.h b/include/linux/psi.h index e074587..8f2db51 100644 --- a/include/linux/psi.h +++ b/include/linux/psi.h @@ -34,7 +34,7 @@ __poll_t psi_trigger_poll(void **trigger_ptr, struct file *file, #ifdef CONFIG_CGROUPS static inline struct psi_group *cgroup_psi(struct cgroup *cgrp) { - return cgroup_ino(cgrp) == 1 ? &psi_system : cgrp->psi; + return cgrp->psi; } int psi_cgroup_alloc(struct cgroup *cgrp); diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index 8f3cef1..07c7747 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -164,7 +164,10 @@ struct cgroup_subsys *cgroup_subsys[] = { static DEFINE_PER_CPU(struct cgroup_rstat_cpu, cgrp_dfl_root_rstat_cpu); /* the default hierarchy */ -struct cgroup_root cgrp_dfl_root = { .cgrp.rstat_cpu = &cgrp_dfl_root_rstat_cpu }; +struct cgroup_root cgrp_dfl_root = { + .cgrp.rstat_cpu = &cgrp_dfl_root_rstat_cpu, + .cgrp.psi = &psi_system, +}; EXPORT_SYMBOL_GPL(cgrp_dfl_root); /*
By initializing the root cgroup's psi field to psi_system, we can consistently obtain the psi information for all cgroups from the struct cgroup. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- include/linux/psi.h | 2 +- kernel/cgroup/cgroup.c | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)