Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <8bda2a8d-7faf-621d-c3c0-6351a49219ea@redhat.com>
Date:   Fri, 14 Jan 2022 15:33:34 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [Question] set_cpus_allowed_ptr() call failed at cpuset_attach()
Content-Language: en-US
To:     Tejun Heo <tj@kernel.org>, Zhang Qiao <zhangqiao22@huawei.com>
Cc:     lizefan.x@bytedance.com, hannes@cmpxchg.org,
        cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
        =?UTF-8?Q?Michal_Koutn=c3=bd?= <mkoutny@suse.com>
References: <09ce5796-798e-83d0-f1a6-ba38a787bfc5@huawei.com>
 <4415cd09-6de3-bb2d-386d-8beb4927fb46@huawei.com>
 <YeGi2yjRi/uZIF/2@slm.duckdns.org>
From:   Waiman Long <longman@redhat.com>
In-Reply-To: <YeGi2yjRi/uZIF/2@slm.duckdns.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 1/14/22 11:20, Tejun Heo wrote:
> (cc'ing Waiman and Michal and quoting whole body)
>
> Seems sane to me but let's hear what Waiman and Michal think.
>
> On Fri, Jan 14, 2022 at 09:15:06AM +0800, Zhang Qiao wrote:
>> Hello everyone
>>
>> 	I found the following warning log on qemu. I migrated a task from one cpuset cgroup to
>> another, while I also performed the cpu hotplug operation, and got following calltrace.
>>
>> 	This may lead to a inconsistency between the affinity of the task and cpuset.cpus of the
>> dest cpuset, but this task can be successfully migrated to the dest cpuset cgroup.
>>
>> 	Can we use cpus_read_lock()/cpus_read_unlock() to guarantee that set_cpus_allowed_ptr()
>> doesn't fail, as follows:
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index d0e163a02099..2535d23d2c51 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -2265,6 +2265,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>>          guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
>>
>>          cgroup_taskset_for_each(task, css, tset) {
>> +               cpus_read_lock();
>>                  if (cs != &top_cpuset)
>>                          guarantee_online_cpus(task, cpus_attach);
>>                  else
>> @@ -2274,6 +2275,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>>                   * fail.  TODO: have a better way to handle failure here
>>                   */
>>                  WARN_ON_ONCE(set_cpus_allowed_ptr(task, cpus_attach));
>> +               cpus_read_unlock();
>>
>>
>> 	Is there a better solution?
>>
>> 	Thanks

The change looks OK to me. However, we may need to run the full set of 
regression test to make sure that lockdep won't complain about potential 
deadlock.

Cheers,
Longman