cgroups commit 61d1d219c4 breaks systemd

cgroups commit 61d1d219c4 breaks systemd

[Jiri Kosina]

Hi,

I am not able to boot current Linus' tree on system which is using 
systemd. In the early boot phase I get

	Failed to create root cgroup hierarchy: No such process
	Failed to allocate manager object: No such process

and that's it, the system is completely unusable afterwards (not having 
any console spawned, no networking, etc).

Bisection pointed to

	commit 61d1d219c4c0761059236a46867bc49943c4d29d
	Author: Mandeep Singh Baines <msb@chromium.org>
	Date:   Mon Jan 30 12:51:56 2012 -0800

	    cgroup: remove extra calls to find_existing_css_set

and I have verified that reverting this single commit on top of current 
Linus' tree makes the problem go away and the system boots properly.

strace output shows that with 61d1d219c applied, systemd is failing while 
trying to add itself to the systemd cgroup:

mkdir("/sys", 0755)                     = -1 EEXIST (File exists)
mkdir("/sys/fs", 0755)                  = -1 EEXIST (File exists)
mkdir("/sys/fs/cgroup", 0755)           = -1 EEXIST (File exists)
mkdir("/sys/fs/cgroup/systemd", 0755)   = -1 EEXIST (File exists)
mkdir("/sys/fs/cgroup/systemd/system", 0755) = 0
open("/sys/fs/cgroup/systemd/system/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 6
fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
write(6, "357\n", 4)                    = -1 ESRCH (No such process)
close(6)                                = 0
munmap(0x7f60c4ecb000, 4096)            = 0
writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to create root cgroup hierarchy: No such process", 55}, {"\n", 1}], 5) = 74
open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
read(6, "357\n", 4096)                  = 4
open("/sys/fs/cgroup/systemd/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 7
fstat(7, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4eca000
write(7, "357\n", 4)                    = -1 ESRCH (No such process)
close(7)                                = 0
munmap(0x7f60c4eca000, 4096)            = 0
read(6, "", 4096)                       = 0
close(6)                                = 0
munmap(0x7f60c4ecb000, 4096)            = 0
open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
read(6, "", 4096)                       = 0
close(6)                                = 0
munmap(0x7f60c4ecb000, 4096)            = 0
open("/sys/fs/cgroup/systemd/system", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 6
getdents(6, /* 7 entries */, 32768)     = 240
getdents(6, /* 0 entries */, 32768)     = 0
lstat("/sys/fs/cgroup/systemd/system/tasks", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
rmdir("/sys/fs/cgroup/systemd/system")  = 0
close(6)                                = 0
close(4)                                = 0
close(5)                                = 0
close(0)                                = 0
writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to allocate manager object: No such process", 50}, {"\n", 1}], 5) = 69

-- 
Jiri Kosina
SUSE Labs

cgroups commit 61d1d219c4 breaks systemd

[Jiri Kosina]

Hi,

I am not able to boot current Linus' tree on system which is using 
systemd. In the early boot phase I get

	Failed to create root cgroup hierarchy: No such process
	Failed to allocate manager object: No such process

and that's it, the system is completely unusable afterwards (not having 
any console spawned, no networking, etc).

Bisection pointed to

	commit 61d1d219c4c0761059236a46867bc49943c4d29d
	Author: Mandeep Singh Baines <msb-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
	Date:   Mon Jan 30 12:51:56 2012 -0800

	    cgroup: remove extra calls to find_existing_css_set

and I have verified that reverting this single commit on top of current 
Linus' tree makes the problem go away and the system boots properly.

strace output shows that with 61d1d219c applied, systemd is failing while 
trying to add itself to the systemd cgroup:

mkdir("/sys", 0755)                     = -1 EEXIST (File exists)
mkdir("/sys/fs", 0755)                  = -1 EEXIST (File exists)
mkdir("/sys/fs/cgroup", 0755)           = -1 EEXIST (File exists)
mkdir("/sys/fs/cgroup/systemd", 0755)   = -1 EEXIST (File exists)
mkdir("/sys/fs/cgroup/systemd/system", 0755) = 0
open("/sys/fs/cgroup/systemd/system/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 6
fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
write(6, "357\n", 4)                    = -1 ESRCH (No such process)
close(6)                                = 0
munmap(0x7f60c4ecb000, 4096)            = 0
writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to create root cgroup hierarchy: No such process", 55}, {"\n", 1}], 5) = 74
open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
read(6, "357\n", 4096)                  = 4
open("/sys/fs/cgroup/systemd/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 7
fstat(7, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4eca000
write(7, "357\n", 4)                    = -1 ESRCH (No such process)
close(7)                                = 0
munmap(0x7f60c4eca000, 4096)            = 0
read(6, "", 4096)                       = 0
close(6)                                = 0
munmap(0x7f60c4ecb000, 4096)            = 0
open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
read(6, "", 4096)                       = 0
close(6)                                = 0
munmap(0x7f60c4ecb000, 4096)            = 0
open("/sys/fs/cgroup/systemd/system", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 6
getdents(6, /* 7 entries */, 32768)     = 240
getdents(6, /* 0 entries */, 32768)     = 0
lstat("/sys/fs/cgroup/systemd/system/tasks", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
rmdir("/sys/fs/cgroup/systemd/system")  = 0
close(6)                                = 0
close(4)                                = 0
close(5)                                = 0
close(0)                                = 0
writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to allocate manager object: No such process", 50}, {"\n", 1}], 5) = 69

-- 
Jiri Kosina
SUSE Labs

Re: cgroups commit 61d1d219c4 breaks systemd

[Tejun Heo]

(using Li's new address and quoting whole message for him)

The change has been in linux-next for quite some time and I've been
booting fedora 16 a lot of times on top of this.  Weird.  I'll try to
find out what's wrong and if I can't spot it quick enough, will send a
revert patch to Linus.  We can try again later.  Mandeep, any ideas?

Thanks.

On Thu, Mar 29, 2012 at 12:01:26AM +0200, Jiri Kosina wrote:
> Hi,
> 
> I am not able to boot current Linus' tree on system which is using 
> systemd. In the early boot phase I get
> 
> 	Failed to create root cgroup hierarchy: No such process
> 	Failed to allocate manager object: No such process
> 
> and that's it, the system is completely unusable afterwards (not having 
> any console spawned, no networking, etc).
> 
> Bisection pointed to
> 
> 	commit 61d1d219c4c0761059236a46867bc49943c4d29d
> 	Author: Mandeep Singh Baines <msb@chromium.org>
> 	Date:   Mon Jan 30 12:51:56 2012 -0800
> 
> 	    cgroup: remove extra calls to find_existing_css_set
> 
> and I have verified that reverting this single commit on top of current 
> Linus' tree makes the problem go away and the system boots properly.
> 
> strace output shows that with 61d1d219c applied, systemd is failing while 
> trying to add itself to the systemd cgroup:
> 
> mkdir("/sys", 0755)                     = -1 EEXIST (File exists)
> mkdir("/sys/fs", 0755)                  = -1 EEXIST (File exists)
> mkdir("/sys/fs/cgroup", 0755)           = -1 EEXIST (File exists)
> mkdir("/sys/fs/cgroup/systemd", 0755)   = -1 EEXIST (File exists)
> mkdir("/sys/fs/cgroup/systemd/system", 0755) = 0
> open("/sys/fs/cgroup/systemd/system/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 6
> fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
> write(6, "357\n", 4)                    = -1 ESRCH (No such process)
> close(6)                                = 0
> munmap(0x7f60c4ecb000, 4096)            = 0
> writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to create root cgroup hierarchy: No such process", 55}, {"\n", 1}], 5) = 74
> open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
> fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
> read(6, "357\n", 4096)                  = 4
> open("/sys/fs/cgroup/systemd/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 7
> fstat(7, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4eca000
> write(7, "357\n", 4)                    = -1 ESRCH (No such process)
> close(7)                                = 0
> munmap(0x7f60c4eca000, 4096)            = 0
> read(6, "", 4096)                       = 0
> close(6)                                = 0
> munmap(0x7f60c4ecb000, 4096)            = 0
> open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
> fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
> read(6, "", 4096)                       = 0
> close(6)                                = 0
> munmap(0x7f60c4ecb000, 4096)            = 0
> open("/sys/fs/cgroup/systemd/system", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 6
> getdents(6, /* 7 entries */, 32768)     = 240
> getdents(6, /* 0 entries */, 32768)     = 0
> lstat("/sys/fs/cgroup/systemd/system/tasks", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> rmdir("/sys/fs/cgroup/systemd/system")  = 0
> close(6)                                = 0
> close(4)                                = 0
> close(5)                                = 0
> close(0)                                = 0
> writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to allocate manager object: No such process", 50}, {"\n", 1}], 5) = 69

-- 
tejun

Re: cgroups commit 61d1d219c4 breaks systemd

[Tejun Heo]

(using Li's new address and quoting whole message for him)

The change has been in linux-next for quite some time and I've been
booting fedora 16 a lot of times on top of this.  Weird.  I'll try to
find out what's wrong and if I can't spot it quick enough, will send a
revert patch to Linus.  We can try again later.  Mandeep, any ideas?

Thanks.

On Thu, Mar 29, 2012 at 12:01:26AM +0200, Jiri Kosina wrote:
> Hi,
> 
> I am not able to boot current Linus' tree on system which is using 
> systemd. In the early boot phase I get
> 
> 	Failed to create root cgroup hierarchy: No such process
> 	Failed to allocate manager object: No such process
> 
> and that's it, the system is completely unusable afterwards (not having 
> any console spawned, no networking, etc).
> 
> Bisection pointed to
> 
> 	commit 61d1d219c4c0761059236a46867bc49943c4d29d
> 	Author: Mandeep Singh Baines <msb-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 	Date:   Mon Jan 30 12:51:56 2012 -0800
> 
> 	    cgroup: remove extra calls to find_existing_css_set
> 
> and I have verified that reverting this single commit on top of current 
> Linus' tree makes the problem go away and the system boots properly.
> 
> strace output shows that with 61d1d219c applied, systemd is failing while 
> trying to add itself to the systemd cgroup:
> 
> mkdir("/sys", 0755)                     = -1 EEXIST (File exists)
> mkdir("/sys/fs", 0755)                  = -1 EEXIST (File exists)
> mkdir("/sys/fs/cgroup", 0755)           = -1 EEXIST (File exists)
> mkdir("/sys/fs/cgroup/systemd", 0755)   = -1 EEXIST (File exists)
> mkdir("/sys/fs/cgroup/systemd/system", 0755) = 0
> open("/sys/fs/cgroup/systemd/system/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 6
> fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
> write(6, "357\n", 4)                    = -1 ESRCH (No such process)
> close(6)                                = 0
> munmap(0x7f60c4ecb000, 4096)            = 0
> writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to create root cgroup hierarchy: No such process", 55}, {"\n", 1}], 5) = 74
> open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
> fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
> read(6, "357\n", 4096)                  = 4
> open("/sys/fs/cgroup/systemd/tasks", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 7
> fstat(7, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4eca000
> write(7, "357\n", 4)                    = -1 ESRCH (No such process)
> close(7)                                = 0
> munmap(0x7f60c4eca000, 4096)            = 0
> read(6, "", 4096)                       = 0
> close(6)                                = 0
> munmap(0x7f60c4ecb000, 4096)            = 0
> open("/sys/fs/cgroup/systemd/system/tasks", O_RDONLY|O_CLOEXEC) = 6
> fstat(6, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f60c4ecb000
> read(6, "", 4096)                       = 0
> close(6)                                = 0
> munmap(0x7f60c4ecb000, 4096)            = 0
> open("/sys/fs/cgroup/systemd/system", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 6
> getdents(6, /* 7 entries */, 32768)     = 240
> getdents(6, /* 0 entries */, 32768)     = 0
> lstat("/sys/fs/cgroup/systemd/system/tasks", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> rmdir("/sys/fs/cgroup/systemd/system")  = 0
> close(6)                                = 0
> close(4)                                = 0
> close(5)                                = 0
> close(0)                                = 0
> writev(3, [{"<27>", 4}, {"systemd", 7}, {"[357]: ", 7}, {"Failed to allocate manager object: No such process", 50}, {"\n", 1}], 5) = 69

-- 
tejun

[PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Tejun Heo]

61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
cgroup_task_migrate() return void.  An unfortunate side effect was
that cgroup_attach_task() was depending on that function's return
value to clear its @retval on the success path.  On cgroup mounts
without any subsystem with ->can_attach() callback,
cgroup_attach_task() ended up returning @retval without initializing
it on success.

For some reason, gcc failed to warn about it and it didn't cause
cgroup_attach_task() to return non-zero value in many cases, probably
due to difference in register allocation.  When the problem
materializes, systemd fails to populate /systemd cgroup mount and
fails to boot.

Fix it by initializing @retval to zero on declaration.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Jiri Kosina <jkosina@suse.cz>
LKML-Reference: <alpine.LNX.2.00.1203282354440.25526@pobox.suse.cz>
---
Jiri, can you please confirm the fix?

Thanks.

 kernel/cgroup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f4ea4b6..ed64cca 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1883,7 +1883,7 @@ static void cgroup_task_migrate(struct cgroup *cgrp, struct cgroup *oldcgrp,
  */
 int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
 {
-	int retval;
+	int retval = 0;
 	struct cgroup_subsys *ss, *failed_ss = NULL;
 	struct cgroup *oldcgrp;
 	struct cgroupfs_root *root = cgrp->root;

[PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Tejun Heo]

61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
cgroup_task_migrate() return void.  An unfortunate side effect was
that cgroup_attach_task() was depending on that function's return
value to clear its @retval on the success path.  On cgroup mounts
without any subsystem with ->can_attach() callback,
cgroup_attach_task() ended up returning @retval without initializing
it on success.

For some reason, gcc failed to warn about it and it didn't cause
cgroup_attach_task() to return non-zero value in many cases, probably
due to difference in register allocation.  When the problem
materializes, systemd fails to populate /systemd cgroup mount and
fails to boot.

Fix it by initializing @retval to zero on declaration.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reported-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
LKML-Reference: <alpine.LNX.2.00.1203282354440.25526-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
---
Jiri, can you please confirm the fix?

Thanks.

 kernel/cgroup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f4ea4b6..ed64cca 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1883,7 +1883,7 @@ static void cgroup_task_migrate(struct cgroup *cgrp, struct cgroup *oldcgrp,
  */
 int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
 {
-	int retval;
+	int retval = 0;
 	struct cgroup_subsys *ss, *failed_ss = NULL;
 	struct cgroup *oldcgrp;
 	struct cgroupfs_root *root = cgrp->root;

Re: [PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Mandeep Singh Baines]

Tejun Heo (tj@kernel.org) wrote:
> 61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
> cgroup_task_migrate() return void.  An unfortunate side effect was
> that cgroup_attach_task() was depending on that function's return
> value to clear its @retval on the success path.  On cgroup mounts
> without any subsystem with ->can_attach() callback,
> cgroup_attach_task() ended up returning @retval without initializing
> it on success.
> 
> For some reason, gcc failed to warn about it and it didn't cause
> cgroup_attach_task() to return non-zero value in many cases, probably
> due to difference in register allocation.  When the problem
> materializes, systemd fails to populate /systemd cgroup mount and
> fails to boot.
> 
> Fix it by initializing @retval to zero on declaration.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>

Reviewed-by: Mandeep Singh Baines <msb@chromium.org>

> Reported-by: Jiri Kosina <jkosina@suse.cz>
> LKML-Reference: <alpine.LNX.2.00.1203282354440.25526@pobox.suse.cz>
> ---
> Jiri, can you please confirm the fix?
> 
> Thanks.
> 
>  kernel/cgroup.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index f4ea4b6..ed64cca 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -1883,7 +1883,7 @@ static void cgroup_task_migrate(struct cgroup *cgrp, struct cgroup *oldcgrp,
>   */
>  int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
>  {
> -	int retval;
> +	int retval = 0;
>  	struct cgroup_subsys *ss, *failed_ss = NULL;
>  	struct cgroup *oldcgrp;
>  	struct cgroupfs_root *root = cgrp->root;

Re: [PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Mandeep Singh Baines]

Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org) wrote:
> 61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
> cgroup_task_migrate() return void.  An unfortunate side effect was
> that cgroup_attach_task() was depending on that function's return
> value to clear its @retval on the success path.  On cgroup mounts
> without any subsystem with ->can_attach() callback,
> cgroup_attach_task() ended up returning @retval without initializing
> it on success.
> 
> For some reason, gcc failed to warn about it and it didn't cause
> cgroup_attach_task() to return non-zero value in many cases, probably
> due to difference in register allocation.  When the problem
> materializes, systemd fails to populate /systemd cgroup mount and
> fails to boot.
> 
> Fix it by initializing @retval to zero on declaration.
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Reviewed-by: Mandeep Singh Baines <msb-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

> Reported-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
> LKML-Reference: <alpine.LNX.2.00.1203282354440.25526-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
> ---
> Jiri, can you please confirm the fix?
> 
> Thanks.
> 
>  kernel/cgroup.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index f4ea4b6..ed64cca 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -1883,7 +1883,7 @@ static void cgroup_task_migrate(struct cgroup *cgrp, struct cgroup *oldcgrp,
>   */
>  int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
>  {
> -	int retval;
> +	int retval = 0;
>  	struct cgroup_subsys *ss, *failed_ss = NULL;
>  	struct cgroup *oldcgrp;
>  	struct cgroupfs_root *root = cgrp->root;

Re: [PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Li Zefan]

Tejun Heo wrote:

> 61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
> cgroup_task_migrate() return void.  An unfortunate side effect was
> that cgroup_attach_task() was depending on that function's return
> value to clear its @retval on the success path.  On cgroup mounts
> without any subsystem with ->can_attach() callback,
> cgroup_attach_task() ended up returning @retval without initializing
> it on success.
> 
> For some reason, gcc failed to warn about it and it didn't cause
> cgroup_attach_task() to return non-zero value in many cases, probably
> due to difference in register allocation.  When the problem
> materializes, systemd fails to populate /systemd cgroup mount and
> fails to boot.
> 
> Fix it by initializing @retval to zero on declaration.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Jiri Kosina <jkosina@suse.cz>
> LKML-Reference: <alpine.LNX.2.00.1203282354440.25526@pobox.suse.cz>


Acked-by: Li Zefan <lizefan@huawei.com>

Re: [PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Li Zefan]

Tejun Heo wrote:

> 61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
> cgroup_task_migrate() return void.  An unfortunate side effect was
> that cgroup_attach_task() was depending on that function's return
> value to clear its @retval on the success path.  On cgroup mounts
> without any subsystem with ->can_attach() callback,
> cgroup_attach_task() ended up returning @retval without initializing
> it on success.
> 
> For some reason, gcc failed to warn about it and it didn't cause
> cgroup_attach_task() to return non-zero value in many cases, probably
> due to difference in register allocation.  When the problem
> materializes, systemd fails to populate /systemd cgroup mount and
> fails to boot.
> 
> Fix it by initializing @retval to zero on declaration.
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Reported-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
> LKML-Reference: <alpine.LNX.2.00.1203282354440.25526-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>


Acked-by: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

Re: [PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Jiri Kosina]

On Thu, 29 Mar 2012, Tejun Heo wrote:

> 61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
> cgroup_task_migrate() return void.  An unfortunate side effect was
> that cgroup_attach_task() was depending on that function's return
> value to clear its @retval on the success path.  On cgroup mounts
> without any subsystem with ->can_attach() callback,
> cgroup_attach_task() ended up returning @retval without initializing
> it on success.
> 
> For some reason, gcc failed to warn about it and it didn't cause
> cgroup_attach_task() to return non-zero value in many cases, probably
> due to difference in register allocation.  When the problem
> materializes, systemd fails to populate /systemd cgroup mount and
> fails to boot.
> 
> Fix it by initializing @retval to zero on declaration.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Jiri Kosina <jkosina@suse.cz>
> LKML-Reference: <alpine.LNX.2.00.1203282354440.25526@pobox.suse.cz>
> ---
> Jiri, can you please confirm the fix?

Thanks Tejun, it works here.

Tested-by: Jiri Kosina <jkosina@suse.cz>

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH for-3.4-fixes] cgroup: cgroup_attach_task() could return -errno after success

[Jiri Kosina]

On Thu, 29 Mar 2012, Tejun Heo wrote:

> 61d1d219c4 "cgroup: remove extra calls to find_existing_css_set" made
> cgroup_task_migrate() return void.  An unfortunate side effect was
> that cgroup_attach_task() was depending on that function's return
> value to clear its @retval on the success path.  On cgroup mounts
> without any subsystem with ->can_attach() callback,
> cgroup_attach_task() ended up returning @retval without initializing
> it on success.
> 
> For some reason, gcc failed to warn about it and it didn't cause
> cgroup_attach_task() to return non-zero value in many cases, probably
> due to difference in register allocation.  When the problem
> materializes, systemd fails to populate /systemd cgroup mount and
> fails to boot.
> 
> Fix it by initializing @retval to zero on declaration.
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Reported-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
> LKML-Reference: <alpine.LNX.2.00.1203282354440.25526-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
> ---
> Jiri, can you please confirm the fix?

Thanks Tejun, it works here.

Tested-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>

-- 
Jiri Kosina
SUSE Labs