On 3/14/23 13:34, Michal Koutný wrote:
Hello Waiman.
On Mon, Mar 06, 2023 at 03:08:46PM -0500, Waiman Long <longman@xxxxxxxxxx> wrote:
- /*
- * Percpu kthreads in top_cpuset are ignored
- */
- if (top_cs && (task->flags & PF_KTHREAD) &&
- kthread_is_per_cpu(task))
- continue;
+ const struct cpumask *possible_mask = task_cpu_possible_mask(task);
- cpumask_and(new_cpus, cs->effective_cpus,
- task_cpu_possible_mask(task));
+ if (top_cs) {
+ /*
+ * Percpu kthreads in top_cpuset are ignored
+ */
+ if ((task->flags & PF_KTHREAD) && kthread_is_per_cpu(task))
+ continue;
+ cpumask_andnot(new_cpus, possible_mask, cs->subparts_cpus);
+ } else {
+ cpumask_and(new_cpus, cs->effective_cpus, possible_mask);
+ }
I'm wrapping my head around this slightly.
1) I'd suggest swapping args in of cpumask_and() to have possible_mask
consistently first.
I don't quite understand what you meant by "swapping args". It is
effective new_cpus = cs->effective_cpus ∩ possible_mask. What is the
point of swapping cs->effective_cpus and possible_mask.
2) Then I'm wondering whether two branches are truly different when
effective_cpus := cpus_allowed - subparts_cpus
top_cpuset.cpus_allowed == possible_mask (1)
effective_cpus may not be equal "cpus_allowed - subparts_cpus" if some
of the CPUs are offline as effective_cpus contains only online CPUs.
subparts_cpu can include offline cpus too. That is why I choose that
expression. I will add a comment to clarify that.
IOW, can you see a difference in what affinities are set to eligible
top_cpuset tasks before and after this patch upon CPU hotplug?
(Hm, (1) holds only in v2. So is this a fix for v1 only?)
This is due to the fact that cpu hotplug code currently doesn't update
the cpu affinity of tasks in the top cpuset. Tasks not in the top cpuset
can rely on the hotplug code to update the cpu affinity appropriately.
For the tasks in the top cpuset, we have to make sure that all the
offline CPUs are included.
Cheers,
Longman
