Lock out users from changing the cpu affinity of those per-cpu system
daemons which cannot survive such a change, such as migration/%d.
Passes basic handtest of sched_setaffinity(2) on various locked and
unlocked processes on a i386, otherwise untested except by eyeball.
Except for one line in i386, no arch needed any changes to support
this patch.
Joe
arch/i386/kernel/apm.c | 1 +
include/linux/sched.h | 1 +
kernel/fork.c | 2 +-
kernel/module.c | 2 ++
kernel/sched.c | 4 ++++
kernel/softirq.c | 4 ++--
kernel/workqueue.c | 1 +
mm/vmscan.c | 5 ++++-
8 files changed, 16 insertions(+), 4 deletions(-)
diff -Nura linux-2.6.0-test2/arch/i386/kernel/apm.c.orig linux-2.6.0-test2/arch/i386/kernel/apm.c
--- linux-2.6.0-test2/arch/i386/kernel/apm.c.orig 2003-07-31 10:36:23.000000000 -0400
+++ linux-2.6.0-test2/arch/i386/kernel/apm.c 2003-07-31 15:52:25.000000000 -0400
@@ -1705,6 +1705,7 @@
* Method suggested by Ingo Molnar.
*/
set_cpus_allowed(current, 1UL << 0);
+ current->flags |= PF_CPULOCK;
BUG_ON(smp_processor_id() != 0);
#endif
diff -Nura linux-2.6.0-test2/include/linux/sched.h.orig linux-2.6.0-test2/include/linux/sched.h
--- linux-2.6.0-test2/include/linux/sched.h.orig 2003-07-27 12:57:39.000000000 -0400
+++ linux-2.6.0-test2/include/linux/sched.h 2003-07-31 15:52:25.000000000 -0400
@@ -488,6 +488,7 @@
#define PF_LESS_THROTTLE 0x01000000 /* Throttle me less: I clena memory */
#define PF_SYNCWRITE 0x00200000 /* I am doing a sync write */
#define PF_READAHEAD 0x00400000 /* I am doing read-ahead */
+#define PF_CPULOCK 0x00800000 /* lock users out from changing cpus_allowed */
#ifdef CONFIG_SMP
extern int set_cpus_allowed(task_t *p, unsigned long new_mask);
diff -Nura linux-2.6.0-test2/kernel/sched.c.orig linux-2.6.0-test2/kernel/sched.c
--- linux-2.6.0-test2/kernel/sched.c.orig 2003-07-27 13:07:14.000000000 -0400
+++ linux-2.6.0-test2/kernel/sched.c 2003-07-31 18:16:20.000000000 -0400
@@ -1930,6 +1930,9 @@
if ((current->euid != p->euid) && (current->euid != p->uid) &&
!capable(CAP_SYS_NICE))
goto out_unlock;
+ if (p->flags & PF_CPULOCK) {
+ goto out_unlock;
+ }
retval = set_cpus_allowed(p, new_mask);
@@ -2380,6 +2383,7 @@
* serially).
*/
set_cpus_allowed(current, 1UL << cpu);
+ current->flags |= PF_CPULOCK;
ret = setscheduler(0, SCHED_FIFO, ¶m);
diff -Nura linux-2.6.0-test2/kernel/softirq.c.orig linux-2.6.0-test2/kernel/softirq.c
--- linux-2.6.0-test2/kernel/softirq.c.orig 2003-07-27 12:58:53.000000000 -0400
+++ linux-2.6.0-test2/kernel/softirq.c 2003-07-31 15:52:25.000000000 -0400
@@ -323,8 +323,8 @@
/* Migrate to the right CPU */
set_cpus_allowed(current, 1UL << cpu);
- if (smp_processor_id() != cpu)
- BUG();
+ current->flags |= PF_CPULOCK;
+ BUG_ON(smp_processor_id() != cpu);
__set_current_state(TASK_INTERRUPTIBLE);
mb();
diff -Nura linux-2.6.0-test2/kernel/module.c.orig linux-2.6.0-test2/kernel/module.c
--- linux-2.6.0-test2/kernel/module.c.orig 2003-07-27 13:06:11.000000000 -0400
+++ linux-2.6.0-test2/kernel/module.c 2003-07-31 15:52:25.000000000 -0400
@@ -483,6 +483,7 @@
setscheduler(current->pid, SCHED_FIFO, ¶m);
#endif
set_cpus_allowed(current, 1UL << (unsigned long)cpu);
+ current->flags |= PF_CPULOCK;
/* Ack: we are alive */
atomic_inc(&stopref_thread_ack);
@@ -544,6 +545,7 @@
/* FIXME: racy with set_cpus_allowed. */
old_allowed = current->cpus_allowed;
set_cpus_allowed(current, 1UL << (unsigned long)cpu);
+ current->flags |= PF_CPULOCK;
atomic_set(&stopref_thread_ack, 0);
stopref_num_threads = 0;
diff -Nura linux-2.6.0-test2/kernel/fork.c.orig linux-2.6.0-test2/kernel/fork.c
--- linux-2.6.0-test2/kernel/fork.c.orig 2003-07-27 12:57:39.000000000 -0400
+++ linux-2.6.0-test2/kernel/fork.c 2003-07-31 15:52:25.000000000 -0400
@@ -705,7 +705,7 @@
{
unsigned long new_flags = p->flags;
- new_flags &= ~PF_SUPERPRIV;
+ new_flags &= ~(PF_SUPERPRIV | PF_CPULOCK);
new_flags |= PF_FORKNOEXEC;
if (!(clone_flags & CLONE_PTRACE))
p->ptrace = 0;
diff -Nura linux-2.6.0-test2/kernel/workqueue.c.orig linux-2.6.0-test2/kernel/workqueue.c
--- linux-2.6.0-test2/kernel/workqueue.c.orig 2003-07-27 13:11:08.000000000 -0400
+++ linux-2.6.0-test2/kernel/workqueue.c 2003-07-31 15:52:25.000000000 -0400
@@ -177,6 +177,7 @@
set_user_nice(current, -10);
set_cpus_allowed(current, 1UL << cpu);
+ current->flags |= PF_CPULOCK;
complete(&startup->done);
diff -Nura linux-2.6.0-test2/mm/vmscan.c.orig linux-2.6.0-test2/mm/vmscan.c
--- linux-2.6.0-test2/mm/vmscan.c.orig 2003-07-27 12:57:44.000000000 -0400
+++ linux-2.6.0-test2/mm/vmscan.c 2003-07-31 15:52:25.000000000 -0400
@@ -960,8 +960,11 @@
daemonize("kswapd%d", pgdat->node_id);
cpumask = node_to_cpumask(pgdat->node_id);
- if (cpumask)
+ if (cpumask) {
set_cpus_allowed(tsk, cpumask);
+ /* FIXME: a node version of PF_CPULOCK would be cool */
+ current->flags |= PF_CPULOCK;
+ }
current->reclaim_state = &reclaim_state;
/*
On Thu, 2003-07-31 at 15:46, Joe Korty wrote:
> Lock out users from changing the cpu affinity of those per-cpu system
> daemons which cannot survive such a change, such as migration/%d.
>
> Passes basic handtest of sched_setaffinity(2) on various locked and
> unlocked processes on a i386, otherwise untested except by eyeball.
>
> Except for one line in i386, no arch needed any changes to support
> this patch.
I have been wondering what to do about processor affinity and kernel
threads. I just concluded "only root can change it, and we can let root
stab herself if she really wants to."
But if this is really an issue, why not just do:
ret = -EINVAL;
if (!p->mm)
goto out_unlock;
in sys_sched_setaffinity ?
Robert Love
Joe Korty <[email protected]> wrote:
>
> Lock out users from changing the cpu affinity of those per-cpu system
> daemons which cannot survive such a change, such as migration/%d.
Generally we prefer to not add code which purely protects root from making
mistakes. Once the sysadmin has nuked his box he'll learn to not do it
again.
Or do you have some deeper reaon for needing this?
> On Thu, 2003-07-31 at 15:46, Joe Korty wrote:
>
> > Lock out users from changing the cpu affinity of those per-cpu system
> > daemons which cannot survive such a change, such as migration/%d.
> >
> > Passes basic handtest of sched_setaffinity(2) on various locked and
> > unlocked processes on a i386, otherwise untested except by eyeball.
> >
> > Except for one line in i386, no arch needed any changes to support
> > this patch.
>
> I have been wondering what to do about processor affinity and kernel
> threads. I just concluded "only root can change it, and we can let root
> stab herself if she really wants to."
>
> But if this is really an issue, why not just do:
>
> ret = -EINVAL;
> if (!p->mm)
> goto out_unlock;
>
> in sys_sched_setaffinity ?
>
> Robert Love
It's not all system daemons, just some of them that need protection.
Keeping the set of locked-down daemons to the smallest possible is
important when one wants to 'set aside' cpus for use only by
specific, need-the-lowest-latency-possible realtime applications.
Joe
On Thu, 2003-07-31 at 16:06, Joe Korty wrote:
> It's not all system daemons, just some of them that need protection.
>
> Keeping the set of locked-down daemons to the smallest possible is
> important when one wants to 'set aside' cpus for use only by
> specific, need-the-lowest-latency-possible realtime applications.
Yah, I know. But this is a lot of code just to prevent root from hanging
herself, and she has plenty of other ways with which to do that.
Robert Love
> Yah, I know. But this is a lot of code just to prevent root from hanging
> herself, and she has plenty of other ways with which to do that.
Actually it is only 20 lines of changes .. 16 lines added, 4 deleted.
Joe
> Joe Korty <[email protected]> wrote:
> >
> > Lock out users from changing the cpu affinity of those per-cpu system
> > daemons which cannot survive such a change, such as migration/%d.
>
> Generally we prefer to not add code which purely protects root from making
> mistakes. Once the sysadmin has nuked his box he'll learn to not do it
> again.
I'd like to be able to write shell scrips that operate on the set of
/proc/[0-9]* without having to know which of the ever-changing list
of processes need to be avoided and which not.
And it's not really root. It's a SYS_CAP_NICE capability. All
realtime apps or their admins will have that capability as a matter
of course in their normal daily operations.
Joe
Joe Korty <[email protected]> writes:
>
> diff -Nura linux-2.6.0-test2/include/linux/sched.h.orig linux-2.6.0-test2/include/linux/sched.h
> --- linux-2.6.0-test2/include/linux/sched.h.orig 2003-07-27 12:57:39.000000000 -0400
> +++ linux-2.6.0-test2/include/linux/sched.h 2003-07-31 15:52:25.000000000 -0400
> @@ -488,6 +488,7 @@
> #define PF_LESS_THROTTLE 0x01000000 /* Throttle me less: I clena memory */
> #define PF_SYNCWRITE 0x00200000 /* I am doing a sync write */
> #define PF_READAHEAD 0x00400000 /* I am doing read-ahead */
> +#define PF_CPULOCK 0x00800000 /* lock users out from changing cpus_allowed */
It would be probably better to just check for ->mm == NULL
This should catch all kernel threads that use daemonize
-Andi
> On Thu, 2003-07-31 at 16:16, Joe Korty wrote:
>
> > Actually it is only 20 lines of changes .. 16 lines added, 4 deleted.
>
> I know. But 16 new lines, including a new process flag, seems overkill.
> That is all I am saying. Just my opinion.
>
> There are a _lot_ of things root can do wrong.
>
> Robert Love
Hi Robert,
I don't consider SYS_CAP_NICE to be a typical 'root' thing at all.
Everything realtime needs it as part of normal operations. I think
of it as an intermediate thing between 'ordinary desktop users' and
'root', and as such it should behave nicely, just like the
plain-vanilla services available to ordinary users.
I believe it is suboptimal to lump everything a normal desktop user
wouldn't normally do as 'root -- let the user beware'.
Joe
On Fri, Aug 01, 2003 at 01:17:34AM +0200, Andi Kleen wrote:
> Joe Korty <[email protected]> writes:
> >
> > diff -Nura linux-2.6.0-test2/include/linux/sched.h.orig linux-2.6.0-test2/include/linux/sched.h
> > --- linux-2.6.0-test2/include/linux/sched.h.orig 2003-07-27 12:57:39.000000000 -0400
> > +++ linux-2.6.0-test2/include/linux/sched.h 2003-07-31 15:52:25.000000000 -0400
> > @@ -488,6 +488,7 @@
> > #define PF_LESS_THROTTLE 0x01000000 /* Throttle me less: I clena memory */
> > #define PF_SYNCWRITE 0x00200000 /* I am doing a sync write */
> > #define PF_READAHEAD 0x00400000 /* I am doing read-ahead */
> > +#define PF_CPULOCK 0x00800000 /* lock users out from changing cpus_allowed */
>
> It would be probably better to just check for ->mm == NULL
>
> This should catch all kernel threads that use daemonize
>
> -Andi
That is what Robert suggested and it is acceptable to me, though suboptimal..it blocks
every daemon, not just the ones that have to be blocked for system survivability.
Joe
On Thu, 2003-07-31 at 16:16, Joe Korty wrote:
> Actually it is only 20 lines of changes .. 16 lines added, 4 deleted.
I know. But 16 new lines, including a new process flag, seems overkill.
That is all I am saying. Just my opinion.
There are a _lot_ of things root can do wrong.
Robert Love
On 08.01, Robert Love wrote:
> On Thu, 2003-07-31 at 16:06, Joe Korty wrote:
>
> > It's not all system daemons, just some of them that need protection.
> >
> > Keeping the set of locked-down daemons to the smallest possible is
> > important when one wants to 'set aside' cpus for use only by
> > specific, need-the-lowest-latency-possible realtime applications.
>
> Yah, I know. But this is a lot of code just to prevent root from hanging
> herself, and she has plenty of other ways with which to do that.
>
Er, why 'she' ?
In spanish we use root==admin as male, and root==tree or plant root,
as female.
Any pointer (like those of jargon dictionaries) ?
TIA ;)
--
J.A. Magallon <[email protected]> \ Software is like
sex:
werewolf.able.es \ It's better when it's
free
Mandrake Linux release 9.2 (Cooker) for i586
Linux 2.4.22-pre9-jam1m (gcc 3.3.1 (Mandrake Linux 9.2 3.3.1-0.7mdk))
On Thu, 2003-07-31 at 16:18, J.A. Magallon wrote:
> Er, why 'she' ?
> In spanish we use root==admin as male, and root==tree or plant root,
> as female.
In English, the pronoun refers to the sex of the person. These is no
gender of words like in Spanish (i.e., a dog is a dog).
So, since administrators come in both sexes, I picked one.
Robert Love
Joe Korty <[email protected]> wrote:
>
> I'd like to be able to write shell scrips that operate on the set of
> /proc/[0-9]* without having to know which of the ever-changing list
> of processes need to be avoided and which not.
Like this?
#!/bin/sh
#
# can_set_affinity pid
#
can_set_affinity()
{
if [ "$(cat /proc/$1/maps)" != "" ]
then
return 0
fi
if head -1 /proc/$1/status | egrep "events|migration"
then
return 1
else
return 0
fi
}
if can_set_affinity $1
then
echo can set affinity of pid $1
else
echo cannot set affinity of pid $1
fi
On Thu, 2003-07-31 at 16:17, Andrew Morton wrote:
> Like this?
You are so damn cunning, Andrew.
Even better, since Joe complained of the "ever changing" list, why not
check for "\[*/[0-9*]\]" ?
Robert Love
On Thu, Jul 31, 2003 at 04:17:03PM -0700, Andrew Morton wrote:
> Joe Korty <[email protected]> wrote:
> >
> > I'd like to be able to write shell scrips that operate on the set of
> > /proc/[0-9]* without having to know which of the ever-changing list
> > of processes need to be avoided and which not.
>
> Like this?
>
> #!/bin/sh
>
> #
> # can_set_affinity pid
> #
> can_set_affinity()
> {
> if [ "$(cat /proc/$1/maps)" != "" ]
> then
> return 0
> fi
> if head -1 /proc/$1/status | egrep "events|migration"
> then
> return 1
> else
> return 0
> fi
> }
>
> if can_set_affinity $1
> then
> echo can set affinity of pid $1
> else
> echo cannot set affinity of pid $1
> fi
Good stuff. I reduced it to (having trouble easily reading the original
output):
name=$(head -1 /proc/$1/status | awk '{print $2}')
echo -n '[' $1 '] ' $name ' '
if [ "$(cat /proc/$1/maps)" == "" ]
then
echo SAFE
else
echo changeable
fi
It catches all those that need catching, plus denies changes to some
daemons that could survive sched_setaffinity: khubd, kirqd, pdflush*,
kswapd0, scsi_eh_[01], ahc_dv_[01], kseriod, and kjournald*.
Joe
On Thu, 31 Jul 2003 15:47:40 -0700, Andrew Morton <[email protected]> wrote:
>Joe Korty <[email protected]> wrote:
>>
>> Lock out users from changing the cpu affinity of those per-cpu system
>> daemons which cannot survive such a change, such as migration/%d.
>
>Generally we prefer to not add code which purely protects root from making
>mistakes. Once the sysadmin has nuked his box he'll learn to not do it
>again.
>
>Or do you have some deeper reaon for needing this?
I have a different reason for wanting something like what Korty's
patch achieves.
My problem is that the perfctr driver needs to enforce CPU affinity
masks on HT P4s due to the fact that they actually are _asymmetric_
MPs (perfctr HW wasn't duplicated as the rest of the state was) which
leads to dynamic scheduling restrictions.
Setting p->cpus_allowed fixes this by replacing the dynamic scheduling
restriction with a static one, which Linux' scheduler handles nicely.
(I'm telling it to use only thread #0 in each processor package.)
The problem is that some _other_ process can change a process'
->cpus_allowed via sys_sched_setaffinity(). Users can shoot themselves,
fine, but in doing so they also mess up the state of the driver and
the perfctrs of other processes.
I could add checks in set_cpus_allowed() and kill the target
process' perfctrs, but another problem is that set_cpus_allowed()
doesn't synchronize with the target process before messing with
its task_struct. So this doesn't work.
Right now my only safe solution is to do additional checks in
switch_to()'s resume path, which slows down context-switches.
/Mikael
Robert Love <[email protected]> writes:
> On Thu, 2003-07-31 at 16:18, J.A. Magallon wrote:
>
> > Er, why 'she' ?
> > In spanish we use root==admin as male, and root==tree or plant root,
> > as female.
>
> In English, the pronoun refers to the sex of the person. These is no
> gender of words like in Spanish (i.e., a dog is a dog).
>
> So, since administrators come in both sexes, I picked one.
Except the convention in English is to use the male in that case.
Using the female pronoun tends to distract from the point.
There are plenty of plural forms that do not imply gender at all.
As in:
> Yah, I know. But this is a lot of code just to prevent root users from hanging
> themselves, and there are plenty of other ways they can do that.
Eric
Eric W. Biederman wrote:
> > So, since administrators come in both sexes, I picked one.
>
> Except the convention in English is to use the male in that case.
> Using the female pronoun tends to distract from the point.
That's right. Many words in English come with a conventional gender,
based on a conventional gender role assignment:
Doctor -> male
Carpenter -> male
Baker -> male
President -> male
Nurse -> female
Maid -> female
Cleaner -> female
Prostitute -> female
etc. I picked some of the more emotive ones for the above list.
It has been shown that then such words are used in stories, without
any mention of one gender or the other, nearly everyone assumes that
the person so described has the conventional gender for the role.
This is not some flimsy, irrelevant observation. The assumption is
strong enough that it is very effective at misleading about who is who
in a story. (Unfortunately I don't have such a story to hand.) And
it most certainly affects who ends up working in which field.
There are words which do not have such a strong gender role these
days, too. Newsreader, for example, would have been strictly male at
one time but not any more. Context matters. Secretary, for example,
would depend on whether you mean a secretary in a small business, or a
secretary of state. Administrator is another: System Administrator or
Network Administrator is quite male (oddly, - as all the sysadmins I
know are female), but Office Administration strikes me as assumedly
female.
As you can see from the latter two examples, a word _itself_ does not
imply a linguistic sex. The role is what is significant.
Many people don't like the stereotyping, and the (significant) social
inequality which is reinforced by this, and choose to do their bit to
alter (possibly correct) the collective view of how things are, or
ought to be.
You're right that it distracts from the point, but then to _not_
distract from the point is to perpetuate social inequality, or
something like that...
> There are plenty of plural forms that do not imply gender at all.
> As in:
>
> > Yah, I know. But this is a lot of code just to prevent root users from hanging
> > themselves, and there are plenty of other ways they can do that.
That is the most neutral. Alas, plurals are often that bit more
cumbersome and dry. Depends whether you're evoking a reference manual
or a personal story, I guess :)
-- Jamie
Jamie Lokier <[email protected]> writes:
> Eric W. Biederman wrote:
> > > So, since administrators come in both sexes, I picked one.
> >
> > Except the convention in English is to use the male in that case.
> > Using the female pronoun tends to distract from the point.
> As you can see from the latter two examples, a word _itself_ does not
> imply a linguistic sex. The role is what is significant.
To some extent. To some extent before people started ``correcting''
the situation the masculine pronoun was on it's way to becoming
gender neutral in English.
In a similar case sir has become vendor neutral in some contexts.
I need to review what has actually happened in the armed services
to see if a vendor neutral usage has caught on at large. But it is
encouraging to see that things like this can happen.
> Many people don't like the stereotyping, and the (significant) social
> inequality which is reinforced by this, and choose to do their bit to
> alter (possibly correct) the collective view of how things are, or
> ought to be.
Same here.
> You're right that it distracts from the point, but then to _not_
> distract from the point is to perpetuate social inequality, or
> something like that...
It lowers my opinion of someone when they do not want to perpetuate
stereo types and yet cannot rephrase a sentence in a gender neutral
way, in English.
> > There are plenty of plural forms that do not imply gender at all.
> > As in:
> >
> > > Yah, I know. But this is a lot of code just to prevent root users from
> hanging
>
> > > themselves, and there are plenty of other ways they can do that.
>
> That is the most neutral. Alas, plurals are often that bit more
> cumbersome and dry. Depends whether you're evoking a reference manual
> or a personal story, I guess :)
So brining this back on topic. Given that plurals are gender neutral,
and that there are gender neutral words like person and people. When doing
kernel documentation please do it in a gender neutral way.
Eric
On Fri, 2003-08-01 at 04:09, Eric W. Biederman wrote:
> Except the convention in English is to use the male in that case.
> Using the female pronoun tends to distract from the point.
Ack, sorry. I will forward your grievance to my English teacher. He
(she?) obviously did a poor job. Or I just do not pay attention.
Robert Love
Eric W. Biederman wrote:
> So bringing this back on topic. Given that plurals are gender
> neutral, and that there are gender neutral words like person and
> people. When doing kernel documentation please do it in a gender
> neutral way.
Agreed.
-- Jamie
> Eric W. Biederman wrote:
> > So bringing this back on topic. Given that plurals are gender
> > neutral, and that there are gender neutral words like person and
> > people. When doing kernel documentation please do it in a gender
> > neutral way.
>
> Agreed.
>
Yes, we are the PC unix after all...