On 2026/1/29 12:03, Chen Ridong wrote:
>
>
> On 2026/1/28 12:42, Waiman Long wrote:
>> The update_isolation_cpumasks() function can be called either directly
>> from regular cpuset control file write with cpuset_full_lock() called
>> or via the CPU hotplug path with cpus_write_lock and cpuset_mutex held.
>>
>> As we are going to enable dynamic update to the nozh_full housekeeping
>> cpumask (HK_TYPE_KERNEL_NOISE) soon with the help of CPU hotplug,
>> allowing the CPU hotplug path to call into housekeeping_update()
>> directly from update_isolation_cpumasks() will cause deadlock. So we
>> have to defer any call to housekeeping_update() after the CPU hotplug
>> operation has finished. This can be done via the task_work_add(...,
>> TWA_RESUME) API where the actual housekeeping_update() call, if needed,
>> will happen right before existing back to userspace.
>>
>> Since the HK_TYPE_DOMAIN housekeeping cpumask should now track the
>> changes in "cpuset.cpus.isolated", add a check in test_cpuset_prs.sh to
>> confirm that the CPU hotplug deferral, if needed, is working as expected.
>>
>> Signed-off-by: Waiman Long <[email protected]>
>> ---
>> kernel/cgroup/cpuset.c | 49 ++++++++++++++++++-
>> .../selftests/cgroup/test_cpuset_prs.sh | 9 ++++
>> 2 files changed, 56 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 7b7d12ab1006..98c7cb732206 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -84,6 +84,10 @@ static cpumask_var_t isolated_cpus;
>> */
>> static bool isolated_cpus_updating;
>>
>> +/* Both cpuset_mutex and cpus_read_locked acquired */
>> +static bool cpuset_full_locked;
>> +static bool isolation_task_work_queued;
>> +
>> /*
>> * A flag to force sched domain rebuild at the end of an operation.
>> * It can be set in
>> @@ -285,10 +289,12 @@ void cpuset_full_lock(void)
>> {
>> cpus_read_lock();
>> mutex_lock(&cpuset_mutex);
>> + cpuset_full_locked = true;
>> }
>>
>> void cpuset_full_unlock(void)
>> {
>> + cpuset_full_locked = false;
>> mutex_unlock(&cpuset_mutex);
>> cpus_read_unlock();
>> }
>> @@ -1285,25 +1291,64 @@ static bool prstate_housekeeping_conflict(int
>> prstate, struct cpumask *new_cpus)
>> return false;
>> }
>>
>> +static void __update_isolation_cpumasks(bool twork);
>> +static void isolation_task_work_fn(struct callback_head *cb)
>> +{
>> + cpuset_full_lock();
>> + __update_isolation_cpumasks(true);
>> + cpuset_full_lock();
>> +}
>> +
>> /*
>> - * update_isolation_cpumasks - Update external isolation related CPU masks
>> + * __update_isolation_cpumasks - Update external isolation related CPU masks
>> + * @twork - set if call from isolation_task_work_fn()
>> *
>> * The following external CPU masks will be updated if necessary:
>> * - workqueue unbound cpumask
>> */
>> -static void update_isolation_cpumasks(void)
>> +static void __update_isolation_cpumasks(bool twork)
>> {
>> int ret;
>>
>> + if (twork)
>> + isolation_task_work_queued = false;
>> +
>> if (!isolated_cpus_updating)
>> return;
>>
>> + /*
>> + * This function can be reached either directly from regular cpuset
>> + * control file write (cpuset_full_locked) or via hotplug
>> + * (cpus_write_lock && cpuset_mutex held). In the later case, we
>> + * defer the housekeeping_update() call to a task_work to avoid
>> + * the possibility of deadlock. The task_work will be run right
>> + * before exiting back to userspace.
>> + */
>> + if (!cpuset_full_locked) {
>> + static struct callback_head twork_cb;
>> +
>> + if (!isolation_task_work_queued) {
>> + init_task_work(&twork_cb, isolation_task_work_fn);
>> + if (!task_work_add(current, &twork_cb, TWA_RESUME))
>> + isolation_task_work_queued = true;
>> + else
>> + /* Current task shouldn't be exiting */
>> + WARN_ON_ONCE(1);
>> + }
>> + return;
>> + }
>> +
>> ret = housekeeping_update(isolated_cpus);
>> WARN_ON_ONCE(ret < 0);
>>
>> isolated_cpus_updating = false;
>> }
>>
>
> The logic is not straightforward; perhaps we can simplify it as follows,
> maybe I missed something, just correct me.
>
> static void isolation_task_work_fn(struct callback_head *cb)
> {
> guard(mutex)(&isolcpus_update_mutex);
> WARN_ON_ONCE(housekeeping_update(isolated_cpus) < 0);
> }
>
> /*
> * __update_isolation_cpumasks - Update external isolation related CPU masks
> * @twork - set if call from isolation_task_work_fn()
> *
> * The following external CPU masks will be updated if necessary:
> * - workqueue unbound cpumask
> */
> static void __update_isolation_cpumasks(bool twork)
> {
> if (!isolated_cpus_updating)
> return;
>
> /*
> * This function can be reached either directly from regular cpuset
> * control file write (cpuset_full_locked) or via hotplug
> * (cpus_write_lock && cpuset_mutex held). In the later case, we
> * defer the housekeeping_update() call to a task_work to avoid
> * the possibility of deadlock. The task_work will be run right
> * before exiting back to userspace.
> */
> if (twork) {
> static struct callback_head twork_cb;
>
> init_task_work(&twork_cb, isolation_task_work_fn);
> if (task_work_add(current, &twork_cb, TWA_RESUME))
> /* Current task shouldn't be exiting */
> WARN_ON_ONCE(1);
>
> return;
> }
>
> lockdep_assert_held(&isolcpus_update_mutex);
> /*
> * Release cpus_read_lock & cpuset_mutex before calling
> * housekeeping_update() and re-acquiring them afterward if not
> * calling from task_work.
> */
>
> cpuset_full_unlock();
> WARN_ON_ONCE(housekeeping_update(isolated_cpus) < 0);
> cpuset_full_lock();
>
> isolated_cpus_updating = false;
> }
>
> static inline void update_isolation_cpumasks(void)
> {
> __update_isolation_cpumasks(false);
> }
>
It can be much clearer:
static void isolation_task_work_fn(struct callback_head *cb)
{
guard(mutex)(&isolcpus_update_mutex);
WARN_ON_ONCE(housekeeping_update(isolated_cpus) < 0);
}
/*
* __update_isolation_cpumasks - Update external isolation related CPU masks
* @defer
*
* The following external CPU masks will be updated if necessary:
* - workqueue unbound cpumask
*/
static void __update_isolation_cpumasks(bool defer)
{
if (!isolated_cpus_updating)
return;
/*
* This function can be reached either directly from regular cpuset
* control file write (cpuset_full_locked) or via hotplug
* (cpus_write_lock && cpuset_mutex held). In the later case, we
* defer the housekeeping_update() call to a task_work to avoid
* the possibility of deadlock. The task_work will be run right
* before exiting back to userspace.
*/
if (defer) {
static struct callback_head twork_cb;
init_task_work(&twork_cb, isolation_task_work_fn);
if (task_work_add(current, &twork_cb, TWA_RESUME))
/* Current task shouldn't be exiting */
WARN_ON_ONCE(1);
return;
}
lockdep_assert_held(&isolcpus_update_mutex);
lockdep_assert_cpus_held();
lockdep_assert_cpuset_lock_held();
/*
* Release cpus_read_lock & cpuset_mutex before calling
* housekeeping_update() and re-acquiring them afterward if not
* calling from task_work.
*/
cpuset_full_unlock();
WARN_ON_ONCE(housekeeping_update(isolated_cpus) < 0);
cpuset_full_lock();
isolated_cpus_updating = false;
}
static inline void update_isolation_cpumasks(void)
{
__update_isolation_cpumasks(false);
}
static inline void asyn_update_isolation_cpumasks(void)
{
__update_isolation_cpumasks(true);
}
The hotplug path just calls asyn_update_isolation_cpumasks(), cpuset_full_locked
and isolation_task_work_queued can be removed.
--
Best regards,
Ridong
