Re: [RFC PATCH] cgroup/cpuset: fix a memory binding failure for cgroup v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/24/22 12:04, Waiman Long wrote:
On 4/21/22 18:22, Tejun Heo wrote:
cc'ing Waiman and copying the whole body.

Waiman, can you please take a look?

Thanks.

On Tue, Apr 19, 2022 at 10:09:58AM +0800, Feng Tang wrote:
We got report that setting cpuset.mems failed when the nodemask
contains a newly onlined memory node (not enumerated during boot)
for cgroup v2, while the binding succeeded for cgroup v1.

The root cause is, for cgroup v2, when a new memory node is onlined,
top_cpuset's 'mem_allowed' is not updated with the new nodemask of
memory nodes, and the following setting memory nodemask will fail,
if the nodemask contains a new node.

Fix it by updating top_cpuset.mems_allowed right after the
new memory node is onlined, just like v1.

Signed-off-by: Feng Tang <feng.tang@xxxxxxxxx>
---
Very likely I missed some details here, but it looks strange that
the top_cpuset.mem_allowed is not updatd even after we onlined
several memory nodes after boot.

  kernel/cgroup/cpuset.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 9390bfd9f1cd..b97caaf16374 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3314,8 +3314,7 @@ static void cpuset_hotplug_workfn(struct work_struct *work)
      /* synchronize mems_allowed to N_MEMORY */
      if (mems_updated) {
          spin_lock_irq(&callback_lock);
-        if (!on_dfl)
-            top_cpuset.mems_allowed = new_mems;
+        top_cpuset.mems_allowed = new_mems;
          top_cpuset.effective_mems = new_mems;
          spin_unlock_irq(&callback_lock);
          update_tasks_nodemask(&top_cpuset);
The on_dfl check was added by commit 7e88291beefb ("cpuset: make cs->{cpus, mems}_allowed as user-configured masks"). This is the expected behavior for cgroup v2 as we don't want to remove a node because it is hot-removed. However, I do see a problem in case we are adding a node that is not originally in top_cpuset.mems_allowed. We should be allowed to add the extra memory node. So something like

        if (!on_dfl)
                top_cpuset.mems_allowed = new_mems;
        else if (!nodes_subset(new_mems, top_cpuset.mems_allowed))
                nodes_or(top_cpuset.mems_allowed, top_cpuset.mems_allowed, new_mems);

For v2, top_cpuset.mems_allowed is set to node_possible_map in cpuset_bind(). Perhaps node_possible_map may not include all the nodes that are hot-pluggable.

I don't know if that is similar problem with cpu_possible_mask or not.


Ah, I know why the top_cpuset.mems_allowed isn't correctly set. There are 2 places where it is set:

 - cpuset_bind(): top_cpuset.mems_allowed = node_possible_map
 - cpuset_init_smp(): top_cpuset.mems_allowed = node_states[N_MEMORY];

The first one is correct, but the second isn't. It turns out that cpuset_init_smp() can be called later than cpuset_bind().

[    2.611207] cpuset_bind called
[    2.631182] cblist_init_generic: Setting adjustable number of callback queues.
[    3.082357] cpuset_init_smp called

So the proper fix may be to make sure that top_cpuset.mems_allowed is initialized correctly.

Cheers,
Longman





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux