Will update patch to account for this.
Best,
Joel Savitz
On Mon, Apr 8, 2019 at 12:00 PM Waiman Long <longman@xxxxxxxxxx> wrote:
>
> On 04/08/2019 11:47 AM, Phil Auld wrote:
> > On Mon, Apr 08, 2019 at 11:39:36AM -0400 Waiman Long wrote:
> >> On 04/08/2019 11:14 AM, Tejun Heo wrote:
> >>> Hello,
> >>>
> >>> (cc'ing Waiman and copying the whole message for him)
> >>>
> >>> On Fri, Apr 05, 2019 at 11:36:59AM -0400, Joel Savitz wrote:
> >>>> If a process is limited by taskset (i.e. cpuset) to only be allowed to
> >>>> run on cpu N, and then cpu N is offlined via hotplug, the process will
> >>>> be assigned the current value of its cpuset cgroup's effective_cpus field
> >>>> in a call to do_set_cpus_allowed() in cpuset_cpus_allowed_fallback().
> >>>> This argument's value does not makes sense for this case, because
> >>>> task_cs(tsk)->effective_cpus is modified by cpuset_hotplug_workfn()
> >>>> to reflect the new value of cpu_active_mask after cpu N is removed from
> >>>> the mask. While this may make sense for the cgroup affinity mask, it
> >>>> does not make sense on a per-task basis, as a task that was previously
> >>>> limited to only be run on cpu N will be limited to every cpu _except_ for
> >>>> cpu N after it is offlined/onlined via hotplug.
> >>>>
> >>>> Pre-patch behavior:
> >>>>
> >>>> $ grep Cpus /proc/$$/status
> >>>> Cpus_allowed: ff
> >>>> Cpus_allowed_list: 0-7
> >>>>
> >>>> $ taskset -p 4 $$
> >>>> pid 19202's current affinity mask: f
> >>>> pid 19202's new affinity mask: 4
> >>>>
> >>>> $ grep Cpus /proc/self/status
> >>>> Cpus_allowed: 04
> >>>> Cpus_allowed_list: 2
> >>>>
> >>>> # echo off > /sys/devices/system/cpu/cpu2/online
> >>>> $ grep Cpus /proc/$$/status
> >>>> Cpus_allowed: 0b
> >>>> Cpus_allowed_list: 0-1,3
> >>>>
> >>>> # echo on > /sys/devices/system/cpu/cpu2/online
> >>>> $ grep Cpus /proc/$$/status
> >>>> Cpus_allowed: 0b
> >>>> Cpus_allowed_list: 0-1,3
> >>>>
> >>>> On a patched system, the final grep produces the following
> >>>> output instead:
> >>>>
> >>>> $ grep Cpus /proc/$$/status
> >>>> Cpus_allowed: ff
> >>>> Cpus_allowed_list: 0-7
> >>>>
> >>>> This patch changes the above behavior by instead simply resetting the mask
> >>>> to cpu_possible_mask.
> >>>>
> >>>> Signed-off-by: Joel Savitz <jsavitz@xxxxxxxxxx>
> >>>> ---
> >>>> kernel/cgroup/cpuset.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> >>>> index 479743db6c37..5f65a2167bdf 100644
> >>>> --- a/kernel/cgroup/cpuset.c
> >>>> +++ b/kernel/cgroup/cpuset.c
> >>>> @@ -3243,7 +3243,7 @@ void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
> >>>> void cpuset_cpus_allowed_fallback(struct task_struct *tsk)
> >>>> {
> >>>> rcu_read_lock();
> >>>> - do_set_cpus_allowed(tsk, task_cs(tsk)->effective_cpus);
> >>>> + do_set_cpus_allowed(tsk, cpu_possible_mask);
> >>>> rcu_read_unlock();
> >>> cpuset directly mangling with per-task masks has always been weird and
> >>> somewhat broken. Given the current cpuset behavior, I suppose this is
> >>> the better behavior. Waiman, what do you think?
> >>>
> >>> Thanks.
> >>>
> >> I think it may be better to use cpus_allowed in the case of fallback to
> >> make sure that the task isn't allowed to run on CPUs it is not supposed
> >> to run on, e.g. in a VM or container under cpuset control. For tasks in
> >> the top cpuset, it is the same as cpu_possible_mask. Of course, we are
> >> assuming that cpus_allowed has some sane value. BTW, there should be
> >> some comments about handling this case of cpu offlining.
> >>
> > This is setting cpus_allowed, so we can't use that here. This is the final
> > fallback. We've already tried parent cpuset bits at this point and found
> > nothing. If the parent had a mask that included a CPU that was still present
> > we would have already used that. I believe Joel's testing included using
> > a cpuset hierarchy and it did the right thing.
> >
> > I don't know if he has those notes still or not.
> >
> >
> > Cheers,
> > Phil
>
> I am referring to "cpus_allowed" in the current cpuset, not the
> cpus_allowed in the task itself. We can add one more fallback within the
> cpuset_cpus_allowed_fallback() that if the current task cpus_allowed is
> the same as cpuset's cpus_allowed, we fall back to cpu_possible_mask.
>
> -Longman
