bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cgroup: Kill the parent controller when its last child is killed
@ 2022-04-03 13:57 Bui Quang Minh
  2022-04-03 15:59 ` kernel test robot
  2022-04-03 17:22 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Bui Quang Minh @ 2022-04-03 13:57 UTC (permalink / raw)
  To: cgroups
  Cc: Bui Quang Minh, Tejun Heo, Zefan Li, Johannes Weiner,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, linux-kernel, netdev, bpf

When umounting a cgroup controller, in case the controller has no children,
the initial ref will be dropped in cgroup_kill_sb. In cgroup_rmdir path,
the controller is deleted from the parent's children list in
css_release_work_fn, which is run on a kernel worker.

With this simple script

	#!/bin/sh

	mount -t cgroup -o none,name=test test ./tmp
	mkdir -p ./tmp/abc

	rmdir ./tmp/abc
	umount ./tmp

	sleep 5
	cat /proc/self/cgroup

The rmdir will remove the last child and umount is expected to kill the
parent controller. However, when running the above script, we may get

	1:name=test:/

This shows that the parent controller has not been killed. The reason is
after rmdir is completed, it is not guaranteed that the parent's children
list is empty as css_release_work_fn is deferred to run on a worker. In
case cgroup_kill_sb is run before that work, it does not drop the initial
ref. Later in the worker, it just removes the child from the list without
checking the list is empty to kill the parent controller. As a result, the
parent controller still has the initial ref but without any logical refs
(children ref, mount ref).

This commit adds a free parent controller path into the worker function to
free up the parent controller when the last child is killed.

Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
---
 kernel/cgroup/cgroup.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index a557eea7166f..220eb1742961 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5157,12 +5157,25 @@ static void css_release_work_fn(struct work_struct *work)
 		container_of(work, struct cgroup_subsys_state, destroy_work);
 	struct cgroup_subsys *ss = css->ss;
 	struct cgroup *cgrp = css->cgroup;
+	struct cgroup *parent = cgroup_parent(cgrp);
 
 	mutex_lock(&cgroup_mutex);
 
 	css->flags |= CSS_RELEASED;
 	list_del_rcu(&css->sibling);
 
+	/*
+	 * If parent doesn't have any children, start killing it.
+	 * And don't kill the default root.
+	 */
+	if (parent && list_empty(&parent->self.children) &&
+	    parent != &cgrp_dfl_root.cgrp &&
+	    !percpu_ref_is_dying(&parent->self.refcnt)) {
+		if (!percpu_ref_is_dying(&cgrp->bpf.refcnt))
+			cgroup_bpf_offline(parent);
+		percpu_ref_kill(&parent->self.refcnt);
+	}
+
 	if (ss) {
 		/* css release path */
 		if (!list_empty(&css->rstat_css_node)) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] cgroup: Kill the parent controller when its last child is killed
  2022-04-03 13:57 [PATCH] cgroup: Kill the parent controller when its last child is killed Bui Quang Minh
@ 2022-04-03 15:59 ` kernel test robot
  2022-04-03 17:22 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-04-03 15:59 UTC (permalink / raw)
  To: Bui Quang Minh, cgroups
  Cc: kbuild-all, Bui Quang Minh, Tejun Heo, Zefan Li, Johannes Weiner,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, linux-kernel, netdev, bpf

Hi Bui,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tj-cgroup/for-next]
[also build test ERROR on v5.17 next-20220401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Bui-Quang-Minh/cgroup-Kill-the-parent-controller-when-its-last-child-is-killed/20220403-215911
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220403/202204032330.l2wsF3mf-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-19) 11.2.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/2bc22feae8a913c7f371bc79ef9967122d8d326c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Bui-Quang-Minh/cgroup-Kill-the-parent-controller-when-its-last-child-is-killed/20220403-215911
        git checkout 2bc22feae8a913c7f371bc79ef9967122d8d326c
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash kernel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   kernel/cgroup/cgroup.c: In function 'css_release_work_fn':
>> kernel/cgroup/cgroup.c:5169:52: error: 'struct cgroup_bpf' has no member named 'refcnt'
    5169 |                 if (!percpu_ref_is_dying(&cgrp->bpf.refcnt))
         |                                                    ^


vim +5169 kernel/cgroup/cgroup.c

  5148	
  5149	static void css_release_work_fn(struct work_struct *work)
  5150	{
  5151		struct cgroup_subsys_state *css =
  5152			container_of(work, struct cgroup_subsys_state, destroy_work);
  5153		struct cgroup_subsys *ss = css->ss;
  5154		struct cgroup *cgrp = css->cgroup;
  5155		struct cgroup *parent = cgroup_parent(cgrp);
  5156	
  5157		mutex_lock(&cgroup_mutex);
  5158	
  5159		css->flags |= CSS_RELEASED;
  5160		list_del_rcu(&css->sibling);
  5161	
  5162		/*
  5163		 * If parent doesn't have any children, start killing it.
  5164		 * And don't kill the default root.
  5165		 */
  5166		if (parent && list_empty(&parent->self.children) &&
  5167		    parent != &cgrp_dfl_root.cgrp &&
  5168		    !percpu_ref_is_dying(&parent->self.refcnt)) {
> 5169			if (!percpu_ref_is_dying(&cgrp->bpf.refcnt))
  5170				cgroup_bpf_offline(parent);
  5171			percpu_ref_kill(&parent->self.refcnt);
  5172		}
  5173	
  5174		if (ss) {
  5175			/* css release path */
  5176			if (!list_empty(&css->rstat_css_node)) {
  5177				cgroup_rstat_flush(cgrp);
  5178				list_del_rcu(&css->rstat_css_node);
  5179			}
  5180	
  5181			cgroup_idr_replace(&ss->css_idr, NULL, css->id);
  5182			if (ss->css_released)
  5183				ss->css_released(css);
  5184		} else {
  5185			struct cgroup *tcgrp;
  5186	
  5187			/* cgroup release path */
  5188			TRACE_CGROUP_PATH(release, cgrp);
  5189	
  5190			cgroup_rstat_flush(cgrp);
  5191	
  5192			spin_lock_irq(&css_set_lock);
  5193			for (tcgrp = cgroup_parent(cgrp); tcgrp;
  5194			     tcgrp = cgroup_parent(tcgrp))
  5195				tcgrp->nr_dying_descendants--;
  5196			spin_unlock_irq(&css_set_lock);
  5197	
  5198			/*
  5199			 * There are two control paths which try to determine
  5200			 * cgroup from dentry without going through kernfs -
  5201			 * cgroupstats_build() and css_tryget_online_from_dir().
  5202			 * Those are supported by RCU protecting clearing of
  5203			 * cgrp->kn->priv backpointer.
  5204			 */
  5205			if (cgrp->kn)
  5206				RCU_INIT_POINTER(*(void __rcu __force **)&cgrp->kn->priv,
  5207						 NULL);
  5208		}
  5209	
  5210		mutex_unlock(&cgroup_mutex);
  5211	
  5212		INIT_RCU_WORK(&css->destroy_rwork, css_free_rwork_fn);
  5213		queue_rcu_work(cgroup_destroy_wq, &css->destroy_rwork);
  5214	}
  5215	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] cgroup: Kill the parent controller when its last child is killed
  2022-04-03 13:57 [PATCH] cgroup: Kill the parent controller when its last child is killed Bui Quang Minh
  2022-04-03 15:59 ` kernel test robot
@ 2022-04-03 17:22 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-04-03 17:22 UTC (permalink / raw)
  To: Bui Quang Minh, cgroups
  Cc: llvm, kbuild-all, Bui Quang Minh, Tejun Heo, Zefan Li,
	Johannes Weiner, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, linux-kernel, netdev, bpf

Hi Bui,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tj-cgroup/for-next]
[also build test ERROR on v5.17 next-20220401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Bui-Quang-Minh/cgroup-Kill-the-parent-controller-when-its-last-child-is-killed/20220403-215911
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next
config: arm-mxs_defconfig (https://download.01.org/0day-ci/archive/20220404/202204040103.yTRTggqu-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c4a1b07d0979e7ff20d7d541af666d822d66b566)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/2bc22feae8a913c7f371bc79ef9967122d8d326c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Bui-Quang-Minh/cgroup-Kill-the-parent-controller-when-its-last-child-is-killed/20220403-215911
        git checkout 2bc22feae8a913c7f371bc79ef9967122d8d326c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> kernel/cgroup/cgroup.c:5169:39: error: no member named 'refcnt' in 'struct cgroup_bpf'
                   if (!percpu_ref_is_dying(&cgrp->bpf.refcnt))
                                             ~~~~~~~~~ ^
   1 error generated.


vim +5169 kernel/cgroup/cgroup.c

  5148	
  5149	static void css_release_work_fn(struct work_struct *work)
  5150	{
  5151		struct cgroup_subsys_state *css =
  5152			container_of(work, struct cgroup_subsys_state, destroy_work);
  5153		struct cgroup_subsys *ss = css->ss;
  5154		struct cgroup *cgrp = css->cgroup;
  5155		struct cgroup *parent = cgroup_parent(cgrp);
  5156	
  5157		mutex_lock(&cgroup_mutex);
  5158	
  5159		css->flags |= CSS_RELEASED;
  5160		list_del_rcu(&css->sibling);
  5161	
  5162		/*
  5163		 * If parent doesn't have any children, start killing it.
  5164		 * And don't kill the default root.
  5165		 */
  5166		if (parent && list_empty(&parent->self.children) &&
  5167		    parent != &cgrp_dfl_root.cgrp &&
  5168		    !percpu_ref_is_dying(&parent->self.refcnt)) {
> 5169			if (!percpu_ref_is_dying(&cgrp->bpf.refcnt))
  5170				cgroup_bpf_offline(parent);
  5171			percpu_ref_kill(&parent->self.refcnt);
  5172		}
  5173	
  5174		if (ss) {
  5175			/* css release path */
  5176			if (!list_empty(&css->rstat_css_node)) {
  5177				cgroup_rstat_flush(cgrp);
  5178				list_del_rcu(&css->rstat_css_node);
  5179			}
  5180	
  5181			cgroup_idr_replace(&ss->css_idr, NULL, css->id);
  5182			if (ss->css_released)
  5183				ss->css_released(css);
  5184		} else {
  5185			struct cgroup *tcgrp;
  5186	
  5187			/* cgroup release path */
  5188			TRACE_CGROUP_PATH(release, cgrp);
  5189	
  5190			cgroup_rstat_flush(cgrp);
  5191	
  5192			spin_lock_irq(&css_set_lock);
  5193			for (tcgrp = cgroup_parent(cgrp); tcgrp;
  5194			     tcgrp = cgroup_parent(tcgrp))
  5195				tcgrp->nr_dying_descendants--;
  5196			spin_unlock_irq(&css_set_lock);
  5197	
  5198			/*
  5199			 * There are two control paths which try to determine
  5200			 * cgroup from dentry without going through kernfs -
  5201			 * cgroupstats_build() and css_tryget_online_from_dir().
  5202			 * Those are supported by RCU protecting clearing of
  5203			 * cgrp->kn->priv backpointer.
  5204			 */
  5205			if (cgrp->kn)
  5206				RCU_INIT_POINTER(*(void __rcu __force **)&cgrp->kn->priv,
  5207						 NULL);
  5208		}
  5209	
  5210		mutex_unlock(&cgroup_mutex);
  5211	
  5212		INIT_RCU_WORK(&css->destroy_rwork, css_free_rwork_fn);
  5213		queue_rcu_work(cgroup_destroy_wq, &css->destroy_rwork);
  5214	}
  5215	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-04-03 17:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-03 13:57 [PATCH] cgroup: Kill the parent controller when its last child is killed Bui Quang Minh
2022-04-03 15:59 ` kernel test robot
2022-04-03 17:22 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).