On 03/19/2018 11:59 AM, Tejun Heo wrote: > Hello, Waiman. > > This looks great. A couple nitpicks below. > >> + 5-3. Cpuset >> + 5.3-1. Cpuset Interface Files > Can we put cpuset below pid? It feels weird to break up cpu, memory > and io as they represent the three major resources and are in a > similar fashion. Sure. I will move it down below pid. >> + cpuset.effective_cpus >> + A read-only multiple values file which exists on non-root >> + cgroups. >> + >> + It lists the onlined CPUs that are actually allowed to be >> + used by tasks within the current cgroup. It is a subset of >> + "cpuset.cpus". Its value will be affected by CPU hotplug >> + events. > Can we do cpuset.cpus.availble which lists the cpus available to the > cgroup instead of the eventual computed mask for the cgroup? That'd > be more useful as it doesn't lose the information by and'ing what's > available with the cgroup's mask and it's trivial to determine the > effective from the two masks. I don't get what you want here. cpus is the cpuset's cpus_allowed mask. effective_cpus is the effective_cpus mask. When you say cpus available to the cgroup, do you mean the cpu_online_mask or the list of cpus from the parent? Or do you just want to change the name to cpus.available instead of effective_cpus? >> + cpuset.effective_mems >> + A read-only multiple values file which exists on non-root >> + cgroups. >> + >> + It lists the onlined memory nodes that are actually allowed >> + to be used by tasks within the current cgroup. It is a subset >> + of "cpuset.mems". Its value will be affected by memory nodes >> + hotplug events. > Ditto. > >> +static struct cftype dfl_files[] = { >> + { >> + .name = "cpus", >> + .seq_show = cpuset_common_seq_show, >> + .write = cpuset_write_resmask, >> + .max_write_len = (100U + 6 * NR_CPUS), >> + .private = FILE_CPULIST, >> + }, > Is it missing CFTYPE_NOT_ON_ROOT? Other files too. Right, I will set CFTYPE_NOT_ON_ROOT to "cpus" and "mems" as we are not supposed to change them in the root. The effective_cpus and effective_mems will be there in the root to show what are available. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html