On 11/30/21 22:56, Waiman Long wrote:

On 11/30/21 12:11, Tejun Heo wrote:

quoted

quoted

     Once becoming a partition root, the following two rules restrict
     what changes can be made to "cpuset.cpus".

     1) The value must be exclusive.
     2) If child cpusets exist, the value must be a superset of what
        are defined in the child cpusets.

     The second rule applies even for "member". Other changes to
     "cpuset.cpus" that do not violate the above rules are always
     allowed.

While it isn't necessarily tied to this series, it's a big no-no to 
restrict
what a parent can do depending on what its descendants are doing. A 
cgroup
higher up in the hierarchy should be able to change configuration 
however it
sees fit as deligation breaks down otherwise.

Maybe you can argue that cpuset is special and shouldn't be subject 
to such
convention but I can't see strong enough justifications especially given
that most of these restrictions can be broken by hotplug operations 
anyway
and thus need code to handle those situations.

These are all pre-existing restrictions before the introduction of 
partition. These are checks done in validate_change(). I am just 
saying out loud the existing behavior. If you think that needs to be 
changed, I am fine with that. However, it will be a separate patch as 
it is not a behavior that is introduced by this series.

Of the 2 restrictions listed above, the exclusivity rule is due to the 
use of CS_CPU_EXCLUSIVE flag. I think it is reasonable as it affects 
only siblings, not the parent.

The second restriction was found during my testing. It is caused by the 
following code in validate_change():

         /* Each of our child cpusets must be a subset of us */
         ret = -EBUSY;
         cpuset_for_each_child(c, css, cur)
                 if (!is_cpuset_subset(c, trial))
                         goto out;

It seems that this code was there since v2.6.12 (the beginning of the 
git era). Later in function, we have

         /* On legacy hierarchy, we must be a subset of our parent 
cpuset. */
         ret = -EACCES;
         if (!is_in_v2_mode() && !is_cpuset_subset(trial, par))
                 goto out;

This is actually a duplicate in the case of legacy hierarchy.

I can add a patch to take out the first code block above which I think 
is where most of your objections are. Then I can remove the 2nd 
restriction in my documentation. I would like to emphasize that this is 
a pre-existing behavior which I just happen to document.

Cheers,
Longman