lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <36685999-17f1-479e-9895-a22ef28bce67@redhat.com>
Date: Sun, 24 Aug 2025 13:07:24 -0400
From: Waiman Long <llong@...hat.com>
To: Chen Ridong <chenridong@...weicloud.com>, tj@...nel.org,
 hannes@...xchg.org, mkoutny@...e.com
Cc: cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
 lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH -next v4 3/3] cpuset: add helpers for cpus read and
 cpuset_mutex locks

On 8/18/25 2:41 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@...wei.com>
>
> cpuset: add helpers for cpus_read_lock and cpuset_mutex locks.
>
> Replace repetitive locking patterns with new helpers:
> - cpuset_full_lock()
> - cpuset_full_unlock()
>
> This makes the code cleaner and ensures consistent lock ordering.
>
> Signed-off-by: Chen Ridong <chenridong@...wei.com>
> ---
>   kernel/cgroup/cpuset-internal.h |  2 ++
>   kernel/cgroup/cpuset-v1.c       | 12 +++----
>   kernel/cgroup/cpuset.c          | 60 +++++++++++++++++++--------------
>   3 files changed, 40 insertions(+), 34 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
> index 75b3aef39231..337608f408ce 100644
> --- a/kernel/cgroup/cpuset-internal.h
> +++ b/kernel/cgroup/cpuset-internal.h
> @@ -276,6 +276,8 @@ int cpuset_update_flag(cpuset_flagbits_t bit, struct cpuset *cs, int turning_on)
>   ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>   				    char *buf, size_t nbytes, loff_t off);
>   int cpuset_common_seq_show(struct seq_file *sf, void *v);
> +void cpuset_full_lock(void);
> +void cpuset_full_unlock(void);
>   
>   /*
>    * cpuset-v1.c
> diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
> index b69a7db67090..12e76774c75b 100644
> --- a/kernel/cgroup/cpuset-v1.c
> +++ b/kernel/cgroup/cpuset-v1.c
> @@ -169,8 +169,7 @@ static int cpuset_write_s64(struct cgroup_subsys_state *css, struct cftype *cft,
>   	cpuset_filetype_t type = cft->private;
>   	int retval = -ENODEV;
>   
> -	cpus_read_lock();
> -	cpuset_lock();
> +	cpuset_full_lock();
>   	if (!is_cpuset_online(cs))
>   		goto out_unlock;
>   
> @@ -184,8 +183,7 @@ static int cpuset_write_s64(struct cgroup_subsys_state *css, struct cftype *cft,
>   		break;
>   	}
>   out_unlock:
> -	cpuset_unlock();
> -	cpus_read_unlock();
> +	cpuset_full_unlock();
>   	return retval;
>   }
>   
> @@ -454,8 +452,7 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
>   	cpuset_filetype_t type = cft->private;
>   	int retval = 0;
>   
> -	cpus_read_lock();
> -	cpuset_lock();
> +	cpuset_full_lock();
>   	if (!is_cpuset_online(cs)) {
>   		retval = -ENODEV;
>   		goto out_unlock;
> @@ -498,8 +495,7 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
>   		break;
>   	}
>   out_unlock:
> -	cpuset_unlock();
> -	cpus_read_unlock();
> +	cpuset_full_unlock();
>   	return retval;
>   }
>   
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index d5588a1fef60..d29f90a28e1e 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -250,6 +250,12 @@ static struct cpuset top_cpuset = {
>   
>   static DEFINE_MUTEX(cpuset_mutex);
>   
> +/**
> + * cpuset_lock - Acquire the global cpuset mutex
> + *
> + * This locks the global cpuset mutex to prevent modifications to cpuset
> + * hierarchy and configurations. This helper is not enough to make modification.
> + */
>   void cpuset_lock(void)
>   {
>   	mutex_lock(&cpuset_mutex);
> @@ -260,6 +266,24 @@ void cpuset_unlock(void)
>   	mutex_unlock(&cpuset_mutex);
>   }
>   
> +/**
> + * cpuset_full_lock - Acquire full protection for cpuset modification
> + *
> + * Takes both CPU hotplug read lock (cpus_read_lock()) and cpuset mutex
> + * to safely modify cpuset data.
> + */
> +void cpuset_full_lock(void)
> +{
> +	cpus_read_lock();
> +	mutex_lock(&cpuset_mutex);
> +}
> +
> +void cpuset_full_unlock(void)
> +{
> +	mutex_unlock(&cpuset_mutex);
> +	cpus_read_unlock();
> +}
> +
>   static DEFINE_SPINLOCK(callback_lock);
>   
>   void cpuset_callback_lock_irq(void)
> @@ -3233,8 +3257,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>   	int retval = -ENODEV;
>   
>   	buf = strstrip(buf);
> -	cpus_read_lock();
> -	mutex_lock(&cpuset_mutex);
> +	cpuset_full_lock();
>   	if (!is_cpuset_online(cs))
>   		goto out_unlock;
>   
> @@ -3263,8 +3286,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>   	if (force_sd_rebuild)
>   		rebuild_sched_domains_locked();
>   out_unlock:
> -	mutex_unlock(&cpuset_mutex);
> -	cpus_read_unlock();
> +	cpuset_full_unlock();
>   	flush_workqueue(cpuset_migrate_mm_wq);
>   	return retval ?: nbytes;
>   }
> @@ -3367,12 +3389,10 @@ static ssize_t cpuset_partition_write(struct kernfs_open_file *of, char *buf,
>   	else
>   		return -EINVAL;
>   
> -	cpus_read_lock();
> -	mutex_lock(&cpuset_mutex);
> +	cpuset_full_lock();
>   	if (is_cpuset_online(cs))
>   		retval = update_prstate(cs, val);
> -	mutex_unlock(&cpuset_mutex);
> -	cpus_read_unlock();
> +	cpuset_full_unlock();
>   	return retval ?: nbytes;
>   }
>   
> @@ -3497,9 +3517,7 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
>   	if (!parent)
>   		return 0;
>   
> -	cpus_read_lock();
> -	mutex_lock(&cpuset_mutex);
> -
> +	cpuset_full_lock();
>   	if (is_spread_page(parent))
>   		set_bit(CS_SPREAD_PAGE, &cs->flags);
>   	if (is_spread_slab(parent))
> @@ -3551,8 +3569,7 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
>   	cpumask_copy(cs->effective_cpus, parent->cpus_allowed);
>   	spin_unlock_irq(&callback_lock);
>   out_unlock:
> -	mutex_unlock(&cpuset_mutex);
> -	cpus_read_unlock();
> +	cpuset_full_unlock();
>   	return 0;
>   }
>   
> @@ -3567,16 +3584,12 @@ static void cpuset_css_offline(struct cgroup_subsys_state *css)
>   {
>   	struct cpuset *cs = css_cs(css);
>   
> -	cpus_read_lock();
> -	mutex_lock(&cpuset_mutex);
> -
> +	cpuset_full_lock();
>   	if (!cpuset_v2() && is_sched_load_balance(cs))
>   		cpuset_update_flag(CS_SCHED_LOAD_BALANCE, cs, 0);
>   
>   	cpuset_dec();
> -
> -	mutex_unlock(&cpuset_mutex);
> -	cpus_read_unlock();
> +	cpuset_full_unlock();
>   }
>   
>   /*
> @@ -3588,16 +3601,11 @@ static void cpuset_css_killed(struct cgroup_subsys_state *css)
>   {
>   	struct cpuset *cs = css_cs(css);
>   
> -	cpus_read_lock();
> -	mutex_lock(&cpuset_mutex);
> -
> +	cpuset_full_lock();
>   	/* Reset valid partition back to member */
>   	if (is_partition_valid(cs))
>   		update_prstate(cs, PRS_MEMBER);
> -
> -	mutex_unlock(&cpuset_mutex);
> -	cpus_read_unlock();
> -
> +	cpuset_full_unlock();
>   }
>   
>   static void cpuset_css_free(struct cgroup_subsys_state *css)
Reviewed-by: Waiman Long <longman@...hat.com>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ