[PATCH] How to write a portable "run_on" program?

[PATCH] How to write a portable "run_on" program?

[Rusty Russell]

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;
 

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

[Linus Torvalds]

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

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

[Rusty Russell]

In message <Pine.LNX.4.44.0305072221120.16772-100000@home.transmeta.com> 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.

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

[Rusty Russell]

In message <Pine.LNX.4.44.0305072221120.16772-100000@home.transmeta.com> 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.