Re: [PATCH v6 1/3] cgroups: use bitmask to filter for_each_subsys

[Tejun Heo <tj@kernel.org>]

Hello,

On Sat, Mar 14, 2015 at 03:37:13PM +1100, Aleksa Sarai wrote:
> -/* This flag indicates whether tasks in the fork and exit paths should
> +/*
> + * This bitmask flag indicates whether tasks in the fork and exit paths should

Please reflow the comment.  It's going over 80col.  Also, wouldn't it
be clearer if the comment actually stated that the bitmask is
subsystem bitmask?

> @@ -4932,7 +4946,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early)
>  	 * init_css_set is in the subsystem's root cgroup. */
>  	init_css_set.subsys[ss->id] = css;
>  
> -	need_forkexit_callback |= ss->fork || ss->exit;
> +	need_forkexit_callback |= !!(ss->fork || ss->exit) << ss->id;

I generally prefer (bool) casts to !!'s these days but this doesn't
really matter.  More importantly, shouldn't we be splitting fork and
exit masks so that we don't have to test the callback existence later?

> @@ -5239,11 +5253,9 @@ void cgroup_post_fork(struct task_struct *child)
>  	 * css_set; otherwise, @child might change state between ->fork()
>  	 * and addition to css_set.
>  	 */
> -	if (need_forkexit_callback) {
> -		for_each_subsys(ss, i)
> -			if (ss->fork)
> -				ss->fork(child);
> -	}
> +	for_each_subsys_which(need_forkexit_callback, ss, i)
> +		if (ss->fork)
> +			ss->fork(child);
>  }

IOW, we should be able to do

	for_each_subsys_which(subsys_has_fork, ss, i)
		ss->fork(child);

And ditto for exit.

Thanks.

-- 
tejun