Message ID | 20210322160253.4032422-7-arnd@kernel.org |
---|---|
State | New |
Headers | show |
Series | [01/11] x86: compressed: avoid gcc-11 -Wstringop-overread warning | expand |
On Mon, Mar 22, 2021 at 05:02:44PM +0100, Arnd Bergmann <arnd@kernel.org> wrote: > I'm not sure what is expected to happen for such a configuration, > presumably these functions are never calls in that case. Yes, the functions you patched would only be called from subsystems or there should be no way to obtain a struct cgroup_subsys reference anyway (hence it's ok to always branch as if ss==NULL). I'd prefer a variant that wouldn't compile the affected codepaths when there are no subsystems registered, however, I couldn't come up with a way how to do it without some preprocessor ugliness. Reviewed-by: Michal Koutný <mkoutny@suse.com>
On Tue, Mar 30, 2021 at 10:41 AM Michal Koutný <mkoutny@suse.com> wrote: > > On Mon, Mar 22, 2021 at 05:02:44PM +0100, Arnd Bergmann <arnd@kernel.org> wrote: > > I'm not sure what is expected to happen for such a configuration, > > presumably these functions are never calls in that case. > Yes, the functions you patched would only be called from subsystems or > there should be no way to obtain a struct cgroup_subsys reference > anyway (hence it's ok to always branch as if ss==NULL). > > I'd prefer a variant that wouldn't compile the affected codepaths when > there are no subsystems registered, however, I couldn't come up with a > way how to do it without some preprocessor ugliness. Would it be possible to enclose most or all of kernel/cgroup/cgroup.c in an #ifdef CGROUP_SUBSYS_COUNT block? I didn't try that myself, but this might be a way to guarantee that there cannot be any callers (it would cause a link error). > Reviewed-by: Michal Koutný <mkoutny@suse.com> Thanks Arnd
On Tue, Mar 30, 2021 at 11:00:36AM +0200, Arnd Bergmann <arnd@kernel.org> wrote: > Would it be possible to enclose most or all of kernel/cgroup/cgroup.c > in an #ifdef CGROUP_SUBSYS_COUNT block? Even without any controllers, there can still be named hierarchies (v1) or the default hierarchy (v2) (for instance) for process tracking purposes. So only parts of kernel/cgroup/cgroup.c could be ifdef'd. Beware that CGROUP_SUBSYS_COUNT is not known at preprocessing stage (you could have a macro alternative though). > I didn't try that myself, but this might be a way to guarantee that > there cannot be any callers (it would cause a link error). Such a guarantee would be nicer, I agree. I tried a bit but anandoned it when I saw macros proliferate (which I found less readable than your current variant). But YMMV. Michal
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index 9153b20e5cc6..3477f1dc7872 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -456,7 +456,7 @@ static u16 cgroup_ss_mask(struct cgroup *cgrp) static struct cgroup_subsys_state *cgroup_css(struct cgroup *cgrp, struct cgroup_subsys *ss) { - if (ss) + if (ss && (CGROUP_SUBSYS_COUNT > 0)) return rcu_dereference_check(cgrp->subsys[ss->id], lockdep_is_held(&cgroup_mutex)); else @@ -534,6 +534,9 @@ struct cgroup_subsys_state *cgroup_e_css(struct cgroup *cgrp, { struct cgroup_subsys_state *css; + if (CGROUP_SUBSYS_COUNT == 0) + return NULL; + do { css = cgroup_css(cgrp, ss); @@ -561,6 +564,9 @@ struct cgroup_subsys_state *cgroup_get_e_css(struct cgroup *cgrp, { struct cgroup_subsys_state *css; + if (CGROUP_SUBSYS_COUNT == 0) + return NULL; + rcu_read_lock(); do { @@ -630,7 +636,7 @@ struct cgroup_subsys_state *of_css(struct kernfs_open_file *of) * the matching css from the cgroup's subsys table is guaranteed to * be and stay valid until the enclosing operation is complete. */ - if (cft->ss) + if (cft->ss && CGROUP_SUBSYS_COUNT > 0) return rcu_dereference_raw(cgrp->subsys[cft->ss->id]); else return &cgrp->self; @@ -2343,6 +2349,9 @@ struct task_struct *cgroup_taskset_next(struct cgroup_taskset *tset, struct css_set *cset = tset->cur_cset; struct task_struct *task = tset->cur_task; + if (CGROUP_SUBSYS_COUNT == 0) + return NULL; + while (&cset->mg_node != tset->csets) { if (!task) task = list_first_entry(&cset->mg_tasks, @@ -4523,7 +4532,7 @@ void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags, it->ss = css->ss; it->flags = flags; - if (it->ss) + if (it->ss && CGROUP_SUBSYS_COUNT > 0) it->cset_pos = &css->cgroup->e_csets[css->ss->id]; else it->cset_pos = &css->cgroup->cset_links;