On Mon, Jul 01, 2019 at 11:59:50PM -0700, Ian Rogers wrote: > @@ -1530,6 +1530,32 @@ perf_event_groups_less(struct perf_event *left, struct > perf_event *right) > if (left->cpu > right->cpu) > return false; > > +#ifdef CONFIG_CGROUP_PERF > + if (left->cgrp != right->cgrp) { > + if (!left->cgrp) > + /* > + * Left has no cgroup but right does, no cgroups come > + * first. > + */ > + return true; > + if (!right->cgrp) > + /* > + * Right has no cgroup but left does, no cgroups come > + * first. > + */ > + return false;
Per CodingStyle any multi-line statement (here due to comments) needs { }. > + if (left->cgrp->css.cgroup != right->cgrp->css.cgroup) { > + if (!left->cgrp->css.cgroup) > + return true; > + if (!right->cgrp->css.cgroup) > + return false; > + /* Two dissimilar cgroups, order by id. */ > + return left->cgrp->css.cgroup->id < > + right->cgrp->css.cgroup->id; > + } This is dis-similar to the existing style ( < true, > false, == fall-through). Can all this be written like: if (!left->cgrp || !left->cgrp.css.cgroup) return true; if (!right->cgrp || !right->cgrp.css.cgroup) return false; if (left->cgrp->css.cgroup->id < right->cgrp->css.cgroup->id) retun true; if (left->cgrp->css.cgroup->id > right->cgrp->css.cgroup->id) return false; ? > + } > +#endif > + > if (left->group_index < right->group_index) > return true; > if (left->group_index > right->group_index)