On 3/14/23 13:34, Michal Koutný wrote:
Hello Waiman.
On Mon, Mar 06, 2023 at 03:08:46PM -0500, Waiman Long longman@redhat.com 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.
- 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.
- 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