Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753001AbbFIElw (ORCPT ); Tue, 9 Jun 2015 00:41:52 -0400 Received: from mail-pa0-f51.google.com ([209.85.220.51]:36283 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751372AbbFIElm (ORCPT ); Tue, 9 Jun 2015 00:41:42 -0400 Date: Tue, 9 Jun 2015 13:41:33 +0900 From: Tejun Heo To: Aleksa Sarai Cc: lizefan@huawei.com, mingo@redhat.com, peterz@infradead.org, richard@nod.at, fweisbec@gmail.com, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org Subject: Re: [PATCH v13 4/5] cgroup: allow a cgroup subsystem to reject a fork Message-ID: <20150609044133.GQ21465@mtj.duckdns.org> References: <1433547736-11707-1-git-send-email-cyphar@cyphar.com> <1433548938-32560-4-git-send-email-cyphar@cyphar.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1433548938-32560-4-git-send-email-cyphar@cyphar.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2719 Lines: 91 Hello, Aleksa. Looks pretty good to me in general. Some minor comments below. On Sat, Jun 06, 2015 at 10:02:17AM +1000, Aleksa Sarai wrote: > diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h > index a593e29..17d0046 100644 > --- a/include/linux/cgroup.h > +++ b/include/linux/cgroup.h > @@ -62,9 +62,15 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns, > struct pid *pid, struct task_struct *tsk); > void cgroup_fork(struct task_struct *p); > -void cgroup_post_fork(struct task_struct *p); > +extern int cgroup_can_fork(struct task_struct *p, > + void *ss_priv[CGROUP_CANFORK_COUNT]); > +extern void cgroup_cancel_fork(struct task_struct *p, > + void *ss_priv[CGROUP_CANFORK_COUNT]); > +extern void cgroup_post_fork(struct task_struct *p, > + void *old_ss_priv[CGROUP_CANFORK_COUNT]); > void cgroup_exit(struct task_struct *p); > > + Is this blank line intentional? > int cgroup_init_early(void); > int cgroup_init(void); ... > @@ -4924,6 +4927,7 @@ static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early) > > have_fork_callback |= (bool)ss->fork << ss->id; > have_exit_callback |= (bool)ss->exit << ss->id; > + have_canfork_callback |= (bool)ss->can_fork << ss->id; Hmmm.... do we still need this mask? We're already restricting iteration pretty heavily. I'd even suggest dropping both have_fork_callback and have_exit_callback too and just put them inside CGROUP_FORK_EXIT_START / STOP although that doesn't belong in this patchset. ... > +static void *subsys_canfork_priv(void *ss_priv[CGROUP_CANFORK_COUNT], int i) > +{ > + void **private; > + if ((private = subsys_canfork_priv_p(ss_priv, i)) != NULL) > + return *private; > + return NULL; > +} void **private = subsys_canfork...; if (private) return *private; return NULL; or even just return private ? *private : NULL; We conventionally don't put assignments in if conditionals. > +void cgroup_cancel_fork(struct task_struct *child, > + void *ss_priv[CGROUP_CANFORK_COUNT]) > +{ > + struct cgroup_subsys *ss; > + int i; > + > + for_each_subsys(ss, i) > + if(ss->cancel_fork) ^ space > + ss->cancel_fork(child, subsys_canfork_priv(ss_priv, i)); > +} > + > +/** > * cgroup_post_fork - called on a new task after adding it to the task list > * @child: the task in question > * Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/