Re: [PATCH v5 1/2] cpuset: Enable cpuset controller in default hierarchy

From: Tejun Heo
Date: Mon Mar 19 2018 - 17:34:04 EST


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.

> + 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.

> + 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.

Thanks.

--
tejun