* [PATCH -next] cgroup/cpuset: add dec_attach_in_progress helper
@ 2024-07-25 1:55 Chen Ridong
2024-07-25 11:01 ` Kamalesh Babulal
0 siblings, 1 reply; 5+ messages in thread
From: Chen Ridong @ 2024-07-25 1:55 UTC (permalink / raw)
To: tj, lizefan.x, hannes, longman, adityakali, sergeh, mkoutny
Cc: cgroups, linux-kernel
There are several functions to decrease attach_in_progress, and they
will wake up cpuset_attach_wq when attach_in_progress is zero. So,
add a helper to make it concise.
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
kernel/cgroup/cpuset.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index d4322619e59a..c241845694ac 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -490,6 +490,17 @@ static inline void check_insane_mems_config(nodemask_t *nodes)
}
}
+/*
+ * decrease cs->attach_in_progress.
+ * wake_up cpuset_attach_wq if cs->attach_in_progress==0.
+ */
+static inline void dec_attach_in_progress(struct cpuset *cs)
+{
+ cs->attach_in_progress--;
+ if (!cs->attach_in_progress)
+ wake_up(&cpuset_attach_wq);
+}
+
/*
* Cgroup v2 behavior is used on the "cpus" and "mems" control files when
* on default hierarchy or when the cpuset_v2_mode flag is set by mounting
@@ -3421,9 +3432,7 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
cs = css_cs(css);
mutex_lock(&cpuset_mutex);
- cs->attach_in_progress--;
- if (!cs->attach_in_progress)
- wake_up(&cpuset_attach_wq);
+ dec_attach_in_progress(cs);
if (cs->nr_migrate_dl_tasks) {
int cpu = cpumask_any(cs->effective_cpus);
@@ -3538,9 +3547,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
reset_migrate_dl_data(cs);
}
- cs->attach_in_progress--;
- if (!cs->attach_in_progress)
- wake_up(&cpuset_attach_wq);
+ dec_attach_in_progress(cs);
mutex_unlock(&cpuset_mutex);
}
@@ -4284,9 +4291,7 @@ static void cpuset_cancel_fork(struct task_struct *task, struct css_set *cset)
return;
mutex_lock(&cpuset_mutex);
- cs->attach_in_progress--;
- if (!cs->attach_in_progress)
- wake_up(&cpuset_attach_wq);
+ dec_attach_in_progress(cs);
mutex_unlock(&cpuset_mutex);
}
@@ -4319,10 +4324,7 @@ static void cpuset_fork(struct task_struct *task)
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
cpuset_attach_task(cs, task);
- cs->attach_in_progress--;
- if (!cs->attach_in_progress)
- wake_up(&cpuset_attach_wq);
-
+ dec_attach_in_progress(cs);
mutex_unlock(&cpuset_mutex);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH -next] cgroup/cpuset: add dec_attach_in_progress helper
2024-07-25 1:55 [PATCH -next] cgroup/cpuset: add dec_attach_in_progress helper Chen Ridong
@ 2024-07-25 11:01 ` Kamalesh Babulal
2024-07-25 11:40 ` chenridong
0 siblings, 1 reply; 5+ messages in thread
From: Kamalesh Babulal @ 2024-07-25 11:01 UTC (permalink / raw)
To: Chen Ridong, tj, lizefan.x, hannes, longman, adityakali, sergeh,
mkoutny
Cc: cgroups, linux-kernel
On 7/25/24 7:25 AM, Chen Ridong wrote:
> There are several functions to decrease attach_in_progress, and they
> will wake up cpuset_attach_wq when attach_in_progress is zero. So,
> add a helper to make it concise.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
> kernel/cgroup/cpuset.c | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index d4322619e59a..c241845694ac 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -490,6 +490,17 @@ static inline void check_insane_mems_config(nodemask_t *nodes)
> }
> }
>
> +/*
> + * decrease cs->attach_in_progress.
> + * wake_up cpuset_attach_wq if cs->attach_in_progress==0.
In the description, adding locking constraint of cpuset_mutex would be helpful.
Something like "cpuset_mutex must be held by the caller."
> + */
> +static inline void dec_attach_in_progress(struct cpuset *cs)
> +{
> + cs->attach_in_progress--;
> + if (!cs->attach_in_progress)
> + wake_up(&cpuset_attach_wq);
> +}
> +
> /*
> * Cgroup v2 behavior is used on the "cpus" and "mems" control files when
> * on default hierarchy or when the cpuset_v2_mode flag is set by mounting
> @@ -3421,9 +3432,7 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
> cs = css_cs(css);
>
> mutex_lock(&cpuset_mutex);
> - cs->attach_in_progress--;
> - if (!cs->attach_in_progress)
> - wake_up(&cpuset_attach_wq);
> + dec_attach_in_progress(cs);
>
> if (cs->nr_migrate_dl_tasks) {
> int cpu = cpumask_any(cs->effective_cpus);
> @@ -3538,9 +3547,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
> reset_migrate_dl_data(cs);
> }
>
> - cs->attach_in_progress--;
> - if (!cs->attach_in_progress)
> - wake_up(&cpuset_attach_wq);
> + dec_attach_in_progress(cs);
>
> mutex_unlock(&cpuset_mutex);
> }
> @@ -4284,9 +4291,7 @@ static void cpuset_cancel_fork(struct task_struct *task, struct css_set *cset)
> return;
>
> mutex_lock(&cpuset_mutex);
> - cs->attach_in_progress--;
> - if (!cs->attach_in_progress)
> - wake_up(&cpuset_attach_wq);
> + dec_attach_in_progress(cs);
> mutex_unlock(&cpuset_mutex);
> }
>
> @@ -4319,10 +4324,7 @@ static void cpuset_fork(struct task_struct *task)
> guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
> cpuset_attach_task(cs, task);
>
> - cs->attach_in_progress--;
> - if (!cs->attach_in_progress)
> - wake_up(&cpuset_attach_wq);
> -
> + dec_attach_in_progress(cs);
> mutex_unlock(&cpuset_mutex);
> }
>
--
Thanks,
Kamalesh
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH -next] cgroup/cpuset: add dec_attach_in_progress helper
2024-07-25 11:01 ` Kamalesh Babulal
@ 2024-07-25 11:40 ` chenridong
2024-07-25 19:07 ` Waiman Long
0 siblings, 1 reply; 5+ messages in thread
From: chenridong @ 2024-07-25 11:40 UTC (permalink / raw)
To: Kamalesh Babulal, tj, lizefan.x, hannes, longman, adityakali,
sergeh, mkoutny
Cc: cgroups, linux-kernel
On 2024/7/25 19:01, Kamalesh Babulal wrote:
>
>
> On 7/25/24 7:25 AM, Chen Ridong wrote:
>> There are several functions to decrease attach_in_progress, and they
>> will wake up cpuset_attach_wq when attach_in_progress is zero. So,
>> add a helper to make it concise.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>> kernel/cgroup/cpuset.c | 28 +++++++++++++++-------------
>> 1 file changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index d4322619e59a..c241845694ac 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -490,6 +490,17 @@ static inline void check_insane_mems_config(nodemask_t *nodes)
>> }
>> }
>>
>> +/*
>> + * decrease cs->attach_in_progress.
>> + * wake_up cpuset_attach_wq if cs->attach_in_progress==0.
>
> In the description, adding locking constraint of cpuset_mutex would be helpful.
> Something like "cpuset_mutex must be held by the caller."
>
Thank you, I will to that.
>> + */
>> +static inline void dec_attach_in_progress(struct cpuset *cs)
>> +{
>> + cs->attach_in_progress--;
>> + if (!cs->attach_in_progress)
>> + wake_up(&cpuset_attach_wq);
>> +}
>> +
>> /*
>> * Cgroup v2 behavior is used on the "cpus" and "mems" control files when
>> * on default hierarchy or when the cpuset_v2_mode flag is set by mounting
>> @@ -3421,9 +3432,7 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
>> cs = css_cs(css);
>>
>> mutex_lock(&cpuset_mutex);
>> - cs->attach_in_progress--;
>> - if (!cs->attach_in_progress)
>> - wake_up(&cpuset_attach_wq);
>> + dec_attach_in_progress(cs);
>>
>> if (cs->nr_migrate_dl_tasks) {
>> int cpu = cpumask_any(cs->effective_cpus);
>> @@ -3538,9 +3547,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>> reset_migrate_dl_data(cs);
>> }
>>
>> - cs->attach_in_progress--;
>> - if (!cs->attach_in_progress)
>> - wake_up(&cpuset_attach_wq);
>> + dec_attach_in_progress(cs);
>>
>> mutex_unlock(&cpuset_mutex);
>> }
>> @@ -4284,9 +4291,7 @@ static void cpuset_cancel_fork(struct task_struct *task, struct css_set *cset)
>> return;
>>
>> mutex_lock(&cpuset_mutex);
>> - cs->attach_in_progress--;
>> - if (!cs->attach_in_progress)
>> - wake_up(&cpuset_attach_wq);
>> + dec_attach_in_progress(cs);
>> mutex_unlock(&cpuset_mutex);
>> }
>>
>> @@ -4319,10 +4324,7 @@ static void cpuset_fork(struct task_struct *task)
>> guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
>> cpuset_attach_task(cs, task);
>>
>> - cs->attach_in_progress--;
>> - if (!cs->attach_in_progress)
>> - wake_up(&cpuset_attach_wq);
>> -
>> + dec_attach_in_progress(cs);
>> mutex_unlock(&cpuset_mutex);
>> }
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH -next] cgroup/cpuset: add dec_attach_in_progress helper
2024-07-25 11:40 ` chenridong
@ 2024-07-25 19:07 ` Waiman Long
2024-07-26 1:00 ` chenridong
0 siblings, 1 reply; 5+ messages in thread
From: Waiman Long @ 2024-07-25 19:07 UTC (permalink / raw)
To: chenridong, Kamalesh Babulal, tj, lizefan.x, hannes, adityakali,
sergeh, mkoutny
Cc: cgroups, linux-kernel
On 7/25/24 07:40, chenridong wrote:
>
>
> On 2024/7/25 19:01, Kamalesh Babulal wrote:
>>
>>
>> On 7/25/24 7:25 AM, Chen Ridong wrote:
>>> There are several functions to decrease attach_in_progress, and they
>>> will wake up cpuset_attach_wq when attach_in_progress is zero. So,
>>> add a helper to make it concise.
>>>
>>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>>> ---
>>> kernel/cgroup/cpuset.c | 28 +++++++++++++++-------------
>>> 1 file changed, 15 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index d4322619e59a..c241845694ac 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -490,6 +490,17 @@ static inline void
>>> check_insane_mems_config(nodemask_t *nodes)
>>> }
>>> }
>>> +/*
>>> + * decrease cs->attach_in_progress.
>>> + * wake_up cpuset_attach_wq if cs->attach_in_progress==0.
>>
>> In the description, adding locking constraint of cpuset_mutex would
>> be helpful.
>> Something like "cpuset_mutex must be held by the caller."
>>
> Thank you, I will to that.
>>> + */
>>> +static inline void dec_attach_in_progress(struct cpuset *cs)
>>> +{
>>> + cs->attach_in_progress--;
>>> + if (!cs->attach_in_progress)
>>> + wake_up(&cpuset_attach_wq);
>>> +}
>>> +
I would suggested a dec_attach_in_progress_locked() and a
dec_attach_in_progress() helpers. The dec_attach_in_progress() helper
acquires the cpuset_mutex and call dec_attach_in_progress_locked().
Inside the dec_attach_in_progress_locked(), you can either add a comment
about requiring cpuset_mutex held or add a
lockdep_assert_held(&cpuset_mutex).
Cheers,
Longman
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH -next] cgroup/cpuset: add dec_attach_in_progress helper
2024-07-25 19:07 ` Waiman Long
@ 2024-07-26 1:00 ` chenridong
0 siblings, 0 replies; 5+ messages in thread
From: chenridong @ 2024-07-26 1:00 UTC (permalink / raw)
To: Waiman Long, Kamalesh Babulal, tj, lizefan.x, hannes, adityakali,
sergeh, mkoutny
Cc: cgroups, linux-kernel
On 2024/7/26 3:07, Waiman Long wrote:
>
> On 7/25/24 07:40, chenridong wrote:
>>
>>
>> On 2024/7/25 19:01, Kamalesh Babulal wrote:
>>>
>>>
>>> On 7/25/24 7:25 AM, Chen Ridong wrote:
>>>> There are several functions to decrease attach_in_progress, and they
>>>> will wake up cpuset_attach_wq when attach_in_progress is zero. So,
>>>> add a helper to make it concise.
>>>>
>>>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>>>> ---
>>>> kernel/cgroup/cpuset.c | 28 +++++++++++++++-------------
>>>> 1 file changed, 15 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>> index d4322619e59a..c241845694ac 100644
>>>> --- a/kernel/cgroup/cpuset.c
>>>> +++ b/kernel/cgroup/cpuset.c
>>>> @@ -490,6 +490,17 @@ static inline void
>>>> check_insane_mems_config(nodemask_t *nodes)
>>>> }
>>>> }
>>>> +/*
>>>> + * decrease cs->attach_in_progress.
>>>> + * wake_up cpuset_attach_wq if cs->attach_in_progress==0.
>>>
>>> In the description, adding locking constraint of cpuset_mutex would
>>> be helpful.
>>> Something like "cpuset_mutex must be held by the caller."
>>>
>> Thank you, I will to that.
>>>> + */
>>>> +static inline void dec_attach_in_progress(struct cpuset *cs)
>>>> +{
>>>> + cs->attach_in_progress--;
>>>> + if (!cs->attach_in_progress)
>>>> + wake_up(&cpuset_attach_wq);
>>>> +}
>>>> +
>
> I would suggested a dec_attach_in_progress_locked() and a
> dec_attach_in_progress() helpers. The dec_attach_in_progress() helper
> acquires the cpuset_mutex and call dec_attach_in_progress_locked().
> Inside the dec_attach_in_progress_locked(), you can either add a comment
> about requiring cpuset_mutex held or add a
> lockdep_assert_held(&cpuset_mutex).
>
> Cheers,
> Longman
>
>
>
Thanks, I will do that in v2.
Ridong
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-26 1:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-25 1:55 [PATCH -next] cgroup/cpuset: add dec_attach_in_progress helper Chen Ridong
2024-07-25 11:01 ` Kamalesh Babulal
2024-07-25 11:40 ` chenridong
2024-07-25 19:07 ` Waiman Long
2024-07-26 1:00 ` chenridong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox