On 2023/4/4 22:31, Michal Hocko wrote:
> [CC cpuset people]
>
>
> This should go into more details about the usecase, testing and ideally
> also spend couple of words about how CONSTRAINT_CPUSET is actually
> implemented because this is not really immediately obvious. An example
> of before/after behavior would have been really nice as well.
>
> You should also go into much more details about how oom victims are
> actually evaluated.
>
> As this is a userspace visible change it should also be documented
> somewhere in Documentation.
>
> I am not really familiar with cpusets internals so I cannot really judge
> cpuset_cgroup_scan_tasks implementation.
>
> The oom report should be explicit about this being CPUSET specific oom
> handling so unexpected behavior could be nailed down to this change so I
> do not see a major concern from the oom POV. Nevertheless it would be
> still good to consider whether this should be an opt-in behavior. I
> personally do not see a major problem because most cpuset deployments I
> have seen tend to be well partitioned so the new behavior makes more
> sense.
>
On 2023/4/5 01:24, Waiman Long wrote:
>
> You will also need to take cpuset_rwsem to make sure that cpusets are
> stable. BTW, the cpuset_cgroup_scan_tasks() name is kind of redundant. I
> will suggest you just name it as cpuset_scan_tasks(). Please also add a
> doctext comment about its purpose and how it should be used.
Thank you all. I will make the following changes and send v3.
1. Provide more details about the use case, testing, and implementation
of CONSTRAINT_CPUSET, including an example of before/after behavior.
2. Provide more details about how OOM victims are evaluated.
3. Document the userspace visible change in Documentation.
4. Add an option /proc/sys/vm/oom_in_cpuset for cpuset oom.
5. Rename cpuset_cgroup_scan_tasks() to cpuset_scan_tasks() and add a
doctext comment about its purpose and how it should be used.
6. Take cpuset_rwsem to ensure that cpusets are stable.