When setting the allowed cpu mask for a given task, if the task is
already bound to certain cpu, after checking the validity of the new
mask of allowed cpus, job is done, and no further efforts needed for
the valid case as well.
Signed-off-by: Hillf Danton <[email protected]>
---
--- a/kernel/sched.c 2011-04-27 11:48:50.000000000 +0800
+++ b/kernel/sched.c 2011-05-06 20:39:58.000000000 +0800
@@ -5899,9 +5899,9 @@ again:
goto out;
}
- if (unlikely((p->flags & PF_THREAD_BOUND) && p != current &&
- !cpumask_equal(&p->cpus_allowed, new_mask))) {
- ret = -EINVAL;
+ if ((p->flags & PF_THREAD_BOUND) && p != current) {
+ if (!cpumask_equal(&p->cpus_allowed, new_mask))
+ ret = -EINVAL;
goto out;
}
On Fri, May 6, 2011 at 8:52 PM, Hillf Danton <[email protected]> wrote:
> When setting the allowed cpu mask for a given task, if the task is
> already bound to certain cpu, after checking the validity of the new
Maybe we don't need to restrict it only on task bound to certain cpu.
> mask of allowed cpus, job is done, and no further efforts needed for
> the valid case as well.
>
> Signed-off-by: Hillf Danton <[email protected]>
> ---
>
> --- a/kernel/sched.c 2011-04-27 11:48:50.000000000 +0800
> +++ b/kernel/sched.c 2011-05-06 20:39:58.000000000 +0800
> @@ -5899,9 +5899,9 @@ again:
> goto out;
> }
>
> - if (unlikely((p->flags & PF_THREAD_BOUND) && p != current &&
> - !cpumask_equal(&p->cpus_allowed, new_mask))) {
> - ret = -EINVAL;
> + if ((p->flags & PF_THREAD_BOUND) && p != current) {
> + if (!cpumask_equal(&p->cpus_allowed, new_mask))
IOW, we could make '!cpumask_equal(&p->cpus_allowed, new_mask)'
be a separated condition. And I don't see any potential problem with it.
Thanks,
Yong
> + ret = -EINVAL;
> goto out;
> }
>
--
Only stand for myself
On Mon, May 9, 2011 at 12:39 PM, Yong Zhang <[email protected]> wrote:
> On Fri, May 6, 2011 at 8:52 PM, Hillf Danton <[email protected]> wrote:
>> When setting the allowed cpu mask for a given task, if the task is
>> already bound to certain cpu, after checking the validity of the new
>
> Maybe we don't need to restrict it only on task bound to certain cpu.
>
Hi Yong
The original code guards, I guess, casual change in the mask of
allowed CPUs, if bounded,
for tasks such as the workers of work queue. So the restriction looks necessary.
thanks
Hillf
On Mon, May 09, 2011 at 08:52:53PM +0800, Hillf Danton wrote:
> On Mon, May 9, 2011 at 12:39 PM, Yong Zhang <[email protected]> wrote:
> > On Fri, May 6, 2011 at 8:52 PM, Hillf Danton <[email protected]> wrote:
> >> When setting the allowed cpu mask for a given task, if the task is
> >> already bound to certain cpu, after checking the validity of the new
> >
> > Maybe we don't need to restrict it only on task bound to certain cpu.
> >
> Hi Yong
>
> The original code guards, I guess, casual change in the mask of
> allowed CPUs, if bounded,
> for tasks such as the workers of work queue. So the restriction looks necessary.
Yeah, that is true; but I don't think we need to go ahead for
unbounded task if cpu_allowed will not be changed.
My thought is like below:
---
Subject: [PATCH] sched: avoid going ahead if cpu_allowed will not be changed
If cpumask_equal(&p->cpus_allowed, new_mask) is true, seems
there is no reason to prevent set_cpus_allowed_ptr() return
directly.
Signed-off-by: Yong Zhang <[email protected]>
---
kernel/sched.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c
index da93381..56bc1fa 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -5946,13 +5946,15 @@ int set_cpus_allowed_ptr(struct task_struct *p, const struct cpumask *new_mask)
rq = task_rq_lock(p, &flags);
+ if (cpumask_equal(&p->cpus_allowed, new_mask))
+ goto out;
+
if (!cpumask_intersects(new_mask, cpu_active_mask)) {
ret = -EINVAL;
goto out;
}
- if (unlikely((p->flags & PF_THREAD_BOUND) && p != current &&
- !cpumask_equal(&p->cpus_allowed, new_mask))) {
+ if (unlikely((p->flags & PF_THREAD_BOUND) && p != current)) {
ret = -EINVAL;
goto out;
}
--
1.7.1
On Mon, May 9, 2011 at 10:07 PM, Yong Zhang <[email protected]> wrote:
> On Mon, May 09, 2011 at 08:52:53PM +0800, Hillf Danton wrote:
>> On Mon, May 9, 2011 at 12:39 PM, Yong Zhang <[email protected]> wrote:
>> > On Fri, May 6, 2011 at 8:52 PM, Hillf Danton <[email protected]> wrote:
>> >> When setting the allowed cpu mask for a given task, if the task is
>> >> already bound to certain cpu, after checking the validity of the new
>> >
>> > Maybe we don't need to restrict it only on task bound to certain cpu.
>> >
>> Hi Yong
>>
>> The original code guards, I guess, casual change in the mask of
>> allowed CPUs, if bounded,
>> for tasks such as the workers of work queue. So the restriction looks necessary.
>
> Yeah, that is true; but I don't think we need to go ahead for
> unbounded task if cpu_allowed will not be changed.
>
> My thought is like below:
>
> ---
> Subject: [PATCH] sched: avoid going ahead if cpu_allowed will not be changed
>
> If cpumask_equal(&p->cpus_allowed, new_mask) is true, seems
> there is no reason to prevent set_cpus_allowed_ptr() return
> directly.
>
> Signed-off-by: Yong Zhang <[email protected]>
Acked-by: Hillf Danton <[email protected]>
> ---
> kernel/sched.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index da93381..56bc1fa 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -5946,13 +5946,15 @@ int set_cpus_allowed_ptr(struct task_struct *p, const struct cpumask *new_mask)
>
> rq = task_rq_lock(p, &flags);
>
> + if (cpumask_equal(&p->cpus_allowed, new_mask))
> + goto out;
> +
> if (!cpumask_intersects(new_mask, cpu_active_mask)) {
> ret = -EINVAL;
> goto out;
> }
>
> - if (unlikely((p->flags & PF_THREAD_BOUND) && p != current &&
> - !cpumask_equal(&p->cpus_allowed, new_mask))) {
> + if (unlikely((p->flags & PF_THREAD_BOUND) && p != current)) {
> ret = -EINVAL;
> goto out;
> }
> --
> 1.7.1
>
>
Commit-ID: db44fc017d5989302713ab4e7f9e922b648f4b59
Gitweb: http://git.kernel.org/tip/db44fc017d5989302713ab4e7f9e922b648f4b59
Author: Yong Zhang <[email protected]>
AuthorDate: Mon, 9 May 2011 22:07:05 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 16 May 2011 11:01:18 +0200
sched: Avoid going ahead if ->cpus_allowed is not changed
If cpumask_equal(&p->cpus_allowed, new_mask) is true, seems
there is no reason to prevent set_cpus_allowed_ptr() return
directly.
Signed-off-by: Yong Zhang <[email protected]>
Acked-by: Hillf Danton <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/20110509140705.GA2219@zhy
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sched.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c
index b8b9a7d..70bec4f 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -5946,13 +5946,15 @@ int set_cpus_allowed_ptr(struct task_struct *p, const struct cpumask *new_mask)
rq = task_rq_lock(p, &flags);
+ if (cpumask_equal(&p->cpus_allowed, new_mask))
+ goto out;
+
if (!cpumask_intersects(new_mask, cpu_active_mask)) {
ret = -EINVAL;
goto out;
}
- if (unlikely((p->flags & PF_THREAD_BOUND) && p != current &&
- !cpumask_equal(&p->cpus_allowed, new_mask))) {
+ if (unlikely((p->flags & PF_THREAD_BOUND) && p != current)) {
ret = -EINVAL;
goto out;
}