[PATCH cgroup/for-3.10] cgroup: make cgroup_path() not print double slashes

[PATCH cgroup/for-3.10] cgroup: make cgroup_path() not print double slashes

[Tejun Heo]

From 277f3d4be79aefe2071d9053a9c7c89c4e5dad30 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Sun, 14 Apr 2013 10:32:19 -0700

While reimplementing cgroup_path(), 65dff759d2 ("cgroup: fix
cgroup_path() vs rename() race") introduced a bug where the path of a
non-root cgroup would have two slahses at the beginning, which is
caused by adding '/' before the name of the root cgroup which is an
empty string.

 $ grep systemd /proc/self/cgroup
 1:name=systemd://user/root/1

Fix it by special casing root cgroup.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
Applying to cgroup/for-3.10.

Thanks.

 kernel/cgroup.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 678a22c..faf55f5 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1811,11 +1811,17 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 	int ret = -ENAMETOOLONG;
 	char *start;
 
+	if (!cgrp->parent) {
+		if (strlcpy(buf, "/", buflen) >= buflen)
+			return -ENAMETOOLONG;
+		return 0;
+	}
+
 	start = buf + buflen - 1;
 	*start = '\0';
 
 	rcu_read_lock();
-	while (cgrp) {
+	do {
 		const char *name = cgroup_name(cgrp);
 		int len;
 
@@ -1824,15 +1830,12 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 			goto out;
 		memcpy(start, name, len);
 
-		if (!cgrp->parent)
-			break;
-
 		if (--start < buf)
 			goto out;
 		*start = '/';
 
 		cgrp = cgrp->parent;
-	}
+	} while (cgrp->parent);
 	ret = 0;
 	memmove(buf, start, buf + buflen - start);
 out:
-- 
1.8.1.4

[PATCH UPDATED cgroup/for-3.10] cgroup: make cgroup_path() not print double slashes

[Tejun Heo]

From da1f296fd2bfd5ad3c53d72a1ece593e821cf374 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Sun, 14 Apr 2013 10:32:19 -0700

While reimplementing cgroup_path(), 65dff759d2 ("cgroup: fix
cgroup_path() vs rename() race") introduced a bug where the path of a
non-root cgroup would have two slahses at the beginning, which is
caused by treating the root cgroup which has the name '/' like
non-root cgroups.

 $ grep systemd /proc/self/cgroup
 1:name=systemd://user/root/1

Fix it by special casing root cgroup case and not looping over it in
the normal path.

Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
The description was wrong.  Updated.

Thanks.

 kernel/cgroup.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 678a22c..faf55f5 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1811,11 +1811,17 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 	int ret = -ENAMETOOLONG;
 	char *start;
 
+	if (!cgrp->parent) {
+		if (strlcpy(buf, "/", buflen) >= buflen)
+			return -ENAMETOOLONG;
+		return 0;
+	}
+
 	start = buf + buflen - 1;
 	*start = '\0';
 
 	rcu_read_lock();
-	while (cgrp) {
+	do {
 		const char *name = cgroup_name(cgrp);
 		int len;
 
@@ -1824,15 +1830,12 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 			goto out;
 		memcpy(start, name, len);
 
-		if (!cgrp->parent)
-			break;
-
 		if (--start < buf)
 			goto out;
 		*start = '/';
 
 		cgrp = cgrp->parent;
-	}
+	} while (cgrp->parent);
 	ret = 0;
 	memmove(buf, start, buf + buflen - start);
 out:
-- 
1.8.1.4

Re: [PATCH cgroup/for-3.10] cgroup: make cgroup_path() not print double slashes

[Li Zefan]

On 2013/4/15 1:37, Tejun Heo wrote:
>>From 277f3d4be79aefe2071d9053a9c7c89c4e5dad30 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Date: Sun, 14 Apr 2013 10:32:19 -0700
> 
> While reimplementing cgroup_path(), 65dff759d2 ("cgroup: fix
> cgroup_path() vs rename() race") introduced a bug where the path of a
> non-root cgroup would have two slahses at the beginning, which is
> caused by adding '/' before the name of the root cgroup which is an
> empty string.
> 

I guess I booted a wrong kernel when testing that patch...

>  $ grep systemd /proc/self/cgroup
>  1:name=systemd://user/root/1
> 
> Fix it by special casing root cgroup.
> 

I made a patch to fix it before the weekend, but didn't post it. It doesn't
treat root-only cgroup specially.

You can apply whichever you like better.

> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
> Applying to cgroup/for-3.10.
> 
> Thanks.
> 
>  kernel/cgroup.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

[PATCH] cgroup: make cgroup_path() not print double slashes

While reimplementing cgroup_path(), 65dff759d2 ("cgroup: fix
cgroup_path() vs rename() race") introduced a bug where the path of a
non-root cgroup would have two slahses at the beginning, which is
caused by treating the root cgroup which has the name '/' like
non-root cgroups.

Fix it by not adding '/' if it's the root cgroup.

 $ grep systemd /proc/self/cgroup
 1:name=systemd://user/root/1

Signed-off-by: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 kernel/cgroup.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 06aeb42..2a28425 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1809,14 +1809,15 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 			goto out;
 		memcpy(start, name, len);
 
-		if (!cgrp->parent)
+		cgrp = cgrp->parent;
+		if (!cgrp)
 			break;
+		if (!cgrp->parent)
+			continue;
 
 		if (--start < buf)
 			goto out;
 		*start = '/';
-
-		cgrp = cgrp->parent;
 	}
 	ret = 0;
 	memmove(buf, start, buf + buflen - start);
-- 
1.8.0.2

Re: [PATCH cgroup/for-3.10] cgroup: make cgroup_path() not print double slashes

[Tejun Heo]

On Mon, Apr 15, 2013 at 11:10:33AM +0800, Li Zefan wrote:
> On 2013/4/15 1:37, Tejun Heo wrote:
> >>From 277f3d4be79aefe2071d9053a9c7c89c4e5dad30 Mon Sep 17 00:00:00 2001
> > From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Date: Sun, 14 Apr 2013 10:32:19 -0700
> > 
> > While reimplementing cgroup_path(), 65dff759d2 ("cgroup: fix
> > cgroup_path() vs rename() race") introduced a bug where the path of a
> > non-root cgroup would have two slahses at the beginning, which is
> > caused by adding '/' before the name of the root cgroup which is an
> > empty string.
> > 
> 
> I guess I booted a wrong kernel when testing that patch...
> 
> >  $ grep systemd /proc/self/cgroup
> >  1:name=systemd://user/root/1
> > 
> > Fix it by special casing root cgroup.
> > 
> 
> I made a patch to fix it before the weekend, but didn't post it. It doesn't
> treat root-only cgroup specially.
> 
> You can apply whichever you like better.

Already applied the special case patch this morning.  Neither seems
much better than each other so I'll just leave it that way.

Thanks!

-- 
tejun