On Mon, 26 Jun 2017, Vikas Shivappa wrote: > Resource groups (ctrl_mon and monitor groups) are represented by > directories in resctrl fs. Add support to remove the directories.
Again. Please split that patch into two parts; seperate ctrl stuff from rmdir and then add monitoring support. > + rdtgrp->flags = RDT_DELETED; > + free_rmid(rdtgrp->rmid); > + > + /* > + * Remove your rmid from the parent ctrl groups list You are not removing a rmid. You remove the group from the parents group list. Please be more accurate with your comments. Wrong comments are worse than no comments. > + WARN_ON(list_empty(&prdtgrp->crdtgrp_list)); > + list_del(&rdtgrp->crdtgrp_list); > +static int rdtgroup_rmdir_ctrl(struct kernfs_node *kn, struct rdtgroup > *rdtgrp) > +{ > + int cpu, closid = rdtgroup_default.closid; > + struct rdtgroup *entry, *tmp; > + struct list_head *llist; *head please. > + cpumask_var_t tmpmask; > + > + if (!zalloc_cpumask_var(&tmpmask, GFP_KERNEL)) > + return -ENOMEM; Allocation/free can be done at the call site for both functions. > +static int rdtgroup_rmdir(struct kernfs_node *kn) > +{ > + struct kernfs_node *parent_kn = kn->parent; > + struct rdtgroup *rdtgrp; > + int ret = 0; > + > + rdtgrp = rdtgroup_kn_lock_live(kn); > + if (!rdtgrp) { > + ret = -EPERM; > + goto out; > + } > + > + if (rdtgrp->type == RDTCTRL_GROUP && parent_kn == rdtgroup_default.kn) > + ret = rdtgroup_rmdir_ctrl(kn, rdtgrp); > + else if (rdtgrp->type == RDTMON_GROUP && > + !strcmp(parent_kn->name, "mon_groups")) > + ret = rdtgroup_rmdir_mon(kn, rdtgrp); > + else > + ret = -EPERM; Like in the other patch, please makes this parseable. Thanks, tglx