From: "Kevin D. Kissell" <kevink@mips.com>
To: "Pavel Kiryukhin" <vksavl@gmail.com>, <linux-mips@linux-mips.org>
Cc: <vksavl@gmail.com>
Subject: Re: [PATCH][MIPS] fix user_cpus_allowed assignment
Date: Mon, 17 Dec 2007 17:40:03 +0100 [thread overview]
Message-ID: <017c01c840cb$7a5049c0$10eca8c0@grendel> (raw)
In-Reply-To: 73cd086a0712170517i146a452exea775f3942c1d5da@mail.gmail.com
This looks to be a correct fix. Long term, we really do need to convince
the scheduler maintainer to provide hooks that will allow hardware-driven
affinity to be integrated with application-driven affinity in a sensible way,
without requiring replication (and replicated maintenence) of the system
call code in private copies like this. I asked for such hooks in sched.c
when it first became apparent that dynamic FPU affinity was desirable,
but was blown off at that time, so, with regret, I perpetrated the local copy
hack. But it's silly, and MIPS can't possibly be the only architecture where
Linux is used in systems with assymmetric resources where adaptive affinity
is useful.
Regards,
Kevin K.
----- Original Message -----
From: "Pavel Kiryukhin" <vksavl@gmail.com>
To: <linux-mips@linux-mips.org>
Cc: <vksavl@gmail.com>
Sent: Monday, December 17, 2007 2:17 PM
Subject: [PATCH][MIPS] fix user_cpus_allowed assignment
> Hi,
> there seems to be a bug in affinity handling for CONFIG_MIPS_MT_FPAFF=y.
> It can be easily reproduced - for two-cpus system set new mask to 4.
> Call fails, but next sched_getaffinity() call returns 0 as current
> mask.
> Simple fix is below.
>
> Signed-off-by: Pavel Kiryukhin <vksavl@gmail.com>
>
> diff --git a/arch/mips/kernel/mips-mt-fpaff.c
> b/arch/mips/kernel/mips-mt-fpaff.cindex 892665b..774f91e 100644
> --- a/arch/mips/kernel/mips-mt-fpaff.c
> +++ b/arch/mips/kernel/mips-mt-fpaff.c
> @@ -87,9 +87,6 @@ asmlinkage long mipsmt_sys_sched_setaffinity(pid_t
> pid, unsigned int len,
> if (retval)
> goto out_unlock;
>
> - /* Record new user-specified CPU set for future reference */
> - p->thread.user_cpus_allowed = new_mask;
> -
> /* Unlock the task list */
> read_unlock(&tasklist_lock);
>
> @@ -104,6 +101,10 @@ asmlinkage long
> mipsmt_sys_sched_setaffinity(pid_t pid, unsigned int len,
> retval = set_cpus_allowed(p, new_mask);
> }
>
> + /* Record new user-specified CPU set for future reference */
> + if (!retval)
> + p->thread.user_cpus_allowed = new_mask;
> +
> out_unlock:
> put_task_struct(p);
> unlock_cpu_hotplug();
>
>
WARNING: multiple messages have this Message-ID (diff)
From: "Kevin D. Kissell" <kevink@mips.com>
To: Pavel Kiryukhin <vksavl@gmail.com>, linux-mips@linux-mips.org
Subject: Re: [PATCH][MIPS] fix user_cpus_allowed assignment
Date: Mon, 17 Dec 2007 17:40:03 +0100 [thread overview]
Message-ID: <017c01c840cb$7a5049c0$10eca8c0@grendel> (raw)
Message-ID: <20071217164003.5K7k-tyx5HeAst7Ist8KsUOcaNeEHucqmni7nsxEt9E@z> (raw)
In-Reply-To: 73cd086a0712170517i146a452exea775f3942c1d5da@mail.gmail.com
This looks to be a correct fix. Long term, we really do need to convince
the scheduler maintainer to provide hooks that will allow hardware-driven
affinity to be integrated with application-driven affinity in a sensible way,
without requiring replication (and replicated maintenence) of the system
call code in private copies like this. I asked for such hooks in sched.c
when it first became apparent that dynamic FPU affinity was desirable,
but was blown off at that time, so, with regret, I perpetrated the local copy
hack. But it's silly, and MIPS can't possibly be the only architecture where
Linux is used in systems with assymmetric resources where adaptive affinity
is useful.
Regards,
Kevin K.
----- Original Message -----
From: "Pavel Kiryukhin" <vksavl@gmail.com>
To: <linux-mips@linux-mips.org>
Cc: <vksavl@gmail.com>
Sent: Monday, December 17, 2007 2:17 PM
Subject: [PATCH][MIPS] fix user_cpus_allowed assignment
> Hi,
> there seems to be a bug in affinity handling for CONFIG_MIPS_MT_FPAFF=y.
> It can be easily reproduced - for two-cpus system set new mask to 4.
> Call fails, but next sched_getaffinity() call returns 0 as current
> mask.
> Simple fix is below.
>
> Signed-off-by: Pavel Kiryukhin <vksavl@gmail.com>
>
> diff --git a/arch/mips/kernel/mips-mt-fpaff.c
> b/arch/mips/kernel/mips-mt-fpaff.cindex 892665b..774f91e 100644
> --- a/arch/mips/kernel/mips-mt-fpaff.c
> +++ b/arch/mips/kernel/mips-mt-fpaff.c
> @@ -87,9 +87,6 @@ asmlinkage long mipsmt_sys_sched_setaffinity(pid_t
> pid, unsigned int len,
> if (retval)
> goto out_unlock;
>
> - /* Record new user-specified CPU set for future reference */
> - p->thread.user_cpus_allowed = new_mask;
> -
> /* Unlock the task list */
> read_unlock(&tasklist_lock);
>
> @@ -104,6 +101,10 @@ asmlinkage long
> mipsmt_sys_sched_setaffinity(pid_t pid, unsigned int len,
> retval = set_cpus_allowed(p, new_mask);
> }
>
> + /* Record new user-specified CPU set for future reference */
> + if (!retval)
> + p->thread.user_cpus_allowed = new_mask;
> +
> out_unlock:
> put_task_struct(p);
> unlock_cpu_hotplug();
>
>
next prev parent reply other threads:[~2007-12-17 16:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-17 13:17 [PATCH][MIPS] fix user_cpus_allowed assignment Pavel Kiryukhin
2007-12-17 16:40 ` Kevin D. Kissell [this message]
2007-12-17 16:40 ` Kevin D. Kissell
2007-12-21 1:11 ` Ralf Baechle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='017c01c840cb$7a5049c0$10eca8c0@grendel' \
--to=kevink@mips.com \
--cc=linux-mips@linux-mips.org \
--cc=vksavl@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox