On 2013/2/26 10:27, Tejun Heo wrote:

On Mon, Feb 25, 2013 at 02:17:49PM +0800, Li Zefan wrote:

quoted

cgroup_name() returns the name of a cgroup and it must be called with
rcu_read_lock() held.

This will be used by cpuset.

Signed-off-by: Li Zefan <redacted>

...

quoted

 /**
+ * cgroup_name - get the name of a cgroup
+ * @cgrp: the cgroup in question
+ *
+ * Must be called with rcu_read_lock() held.
+ */
+char *cgroup_name(const struct cgroup *cgrp)
+{
+	if (!cgrp->parent)
+		return "/";
+	else
+		return rcu_dereference(cgrp->name)->name;
+}

Can't we initialize ->name of root cgroup to "/" and lose the
conditional? 

Sure we can. We'll have to allocate cgrp->name in cgroup_remount() and
cgroup_init(), and free cgrp->name in cgroup_kill_sb(). It looks to me
the current version is a bit simpler.

That said, I don't have strong preference. I'll revise the patchset if
you still prefer to init root_cgrp->name.

We can lose the wrapper altogether but if you're worried
that sparse check isn't enough, we can have trivial inline wrapper,
but in that case it probably would help to rename cgrp->name to, say,
cgrp->__name and put a comment directing people to the accessing
wrapper which should probably return const char *.

I do expect people always use cgroup_name(). Should anyone access
cgrp->name directly and doesn't notice cgrp->name can be NULL, he'll
get NULL ptr crash and turn to cgroup_name(), and a comment to guide
people to cgroup_name() is helpful too.