2003-05-08 05:03:04

by Rusty Russell

[permalink] [raw]
Subject: [PATCH] How to write a portable "run_on" program?

Hi all,

Currently you need to do the following in userspace to set
your affinity portably:

#include <limits.h>

#define BITS_PER_LONG (sizeof(long)*CHAR_BIT)

int set_cpu(int cpu)
{
size_t size = sizeof(unsigned long);
unsigned long *bitmask = NULL;
int ret;

do {
size *= 2;
bitmask = realloc(bitmask, size);
memset(bitmask, 0, size);
bitmask[cpu / BITS_PER_LONG] = (1 << (cpu % BITS_PER_LONG);
ret = sched_setaffinity(getpid(), size, bitmask);
} while (ret < 0 && errno = -EINVAL);
return ret;
}

I know Linus thinks this is natural and fine[1], but for the rest of
us, how about the following patch, which allows:

int set_cpu(int cpu)
{
size_t size = sched_getaffinity(getpid(), 0, NULL);
unsigned char bitmask[size];

memset(bitmask, 0, size);
bitmask[cpu/8] = (1 << (cpu%8));
return sched_setaffinity(getpid(), size, bitmask);
}

Thoughts?
Rusty.

[1] http://www.ussg.iu.edu/hypermail/linux/kernel/0206.2/0551.html
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.69-bk2/kernel/sched.c working-2.5.69-bk2-getaffinity/kernel/sched.c
--- linux-2.5.69-bk2/kernel/sched.c 2003-05-05 12:37:13.000000000 +1000
+++ working-2.5.69-bk2-getaffinity/kernel/sched.c 2003-05-08 15:11:30.000000000 +1000
@@ -1949,6 +1949,10 @@ asmlinkage long sys_sched_getaffinity(pi
task_t *p;

real_len = sizeof(mask);
+
+ /* This lets the user query the bitmask size. */
+ if (!len)
+ return real_len;
if (len < real_len)
return -EINVAL;


2003-05-08 06:54:13

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] How to write a portable "run_on" program?

In message <[email protected]> you write:
> In other words, the proper way to do set_cpu() is
>
> int set_cpu(int cpu)
> {
> fd_set cpuset;
>
> FD_ZERO(&cpuset);
> FD_SET(cpu, cpuset);
> return sched_setaffinity(getpid(), sizeof(cpuset), &cpuset);
> }
>
> which is a HELL OF A LOT more readable than the crap you're spouting
> (either your "before" _or_ your "after" crap).

Sorry, I remember your solution now. You're absolutely right (still).

I preferred the "include/exclude" flag, which covers the "what to do
when cpus come online/go offline" as well as the size issue, but
that's personal preference.

However, I shall dutifully attack the glibc people for this, then
(/usr/include/sched.h):

#ifdef __USE_GNU
/* Set the CPU affinity for a task */
extern int sched_setaffinity (__pid_t __pid, unsigned long int __len,
unsigned long int *__mask) __THROW;

/* Get the CPU affinity for a task */
extern int sched_getaffinity (__pid_t __pid, unsigned long int __len,
unsigned long int *__mask) __THROW;
#endif

Thanks!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

2003-05-08 06:54:14

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] How to write a portable "run_on" program?

In message <[email protected]> you write:
> In other words, the proper way to do set_cpu() is
>
> int set_cpu(int cpu)
> {
> fd_set cpuset;
>
> FD_ZERO(&cpuset);
> FD_SET(cpu, cpuset);
> return sched_setaffinity(getpid(), sizeof(cpuset), &cpuset);
> }
>
> which is a HELL OF A LOT more readable than the crap you're spouting
> (either your "before" _or_ your "after" crap).

Congratulations: you're the only programmer to get this right. It's
not your code I'm worried about 8(

Meanwhile, everyone is doing "unsigned long" because it "works on
x86".

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

2003-05-08 05:19:28

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] How to write a portable "run_on" program?


On Thu, 8 May 2003, Rusty Russell wrote:
> Currently you need to do the following in userspace to set your
> affinity portably:

This is not true, and as long as you continue to claim this, nobody sane
can take you seriously.

I have several times told you that the proper way to do it is the same as
for fd_set, and in fact you can use "fdset" as the CPU set _today_, even
if it looks a bit strange.

In other words, the proper way to do set_cpu() is

int set_cpu(int cpu)
{
fd_set cpuset;

FD_ZERO(&cpuset);
FD_SET(cpu, cpuset);
return sched_setaffinity(getpid(), sizeof(cpuset), &cpuset);
}

which is a HELL OF A LOT more readable than the crap you're spouting
(either your "before" _or_ your "after" crap).

And if you want to make it more readable still, you make a "cpuset.h"
header file that creates a "cpu_set" type instead of "fd_set". You can
do it by search-and-replace of the fd_set stuff if you want to.

But yeah, if you by "portable" mean that it won't handle more than 1024
CPUs, then I guess it isn't portable. But guess what? I don't care. If you
seriously expect to have more than 1024 CPUs in the near future, make the
cpu_set be bigger.

But I expect you to ignore this email too, the way you ignored my previous
ones. The same way I'll continue to ignore your idiotic patches.

Linus