proc-based cpu affinity user interface

Attached is my procfs-based implementation of a user interface for getting and setting a task's CPU affinity. Patch is against 2.4.16.

The kernel already respects affinity, which is stored in task_struct->cpus_allowed.

Reading and writing /proc/<pid>/affinity will get and set the affinity.

Security is implemented: the writer must possess CAP_SYS_NICE or be the same uid as the task in question. Anyone can read the data.

The read mask will be ANDed with cpu_online_map, so that only valid bits are returned. The written data must have _some_ valid bits in it. I.e., ffffffff is valid on a 2-way system but 01000000 probably is not. Note you don't need to pass the full mask, e.g. "64" is legal. When a new mask is set, a reschedule is forced to put the task on a legal CPU.

Note I had to implement a proc_write function for the procfs (pid) code. This is generic and can be used by other, writable, entries.

This patch comes as an alternative to Ingo Molnar's syscall-implemented variant. Ingo's code is good; however I and others expressed discontent at yet another group of syscalls. Other benefits include the ease with which to set the affinity of tasks that are unaware of the new interface and that with this approach applications don't need to check for the existence of a syscall.

Comments?

Robert Love

diff -urN linux-2.4.16/fs/proc/array.c linux/fs/proc/array.c
@@ -50,6 +50,8 @@
  * Al Viro & Jeff Garzik :  moved most of the thing into base.c and
  *			 :  proc_misc.c. The rest may eventually go into
  *			 :  base.c too.
+ *
+ * Robert Love	     :	added affinity (set processes's CPU affinity)
  */
 
 #include <linux/config.h>

@@ -693,4 +695,64 @@
 
 	return len;
 }
-#endif
+
+int proc_pid_affinity_read(struct task_struct *task, char * buffer)
+{
+	int len;
+
+	/*
+	 * AND the mask against the current CPU configuration to return
+	 * only valid bits
+	 */
+	len = sprintf(buffer, "%08lx\n", task->cpus_allowed & cpu_online_map);
+	return len;
+}
+
+int proc_pid_affinity_write(struct task_struct *task, char * buffer,
+		size_t bytes)
+{
+	unsigned long new_mask;
+	int reschedule = 0;
+	char * endp;
+
+	if ((current->euid != task->euid) && (current->euid != task->uid)
+			&& !capable(CAP_SYS_NICE))
+		return -EPERM;
+
+	new_mask = simple_strtoul(buffer, &endp, 16);
+
+	new_mask &= cpu_online_map;
+	if (!new_mask)
+		return -EINVAL;
+
+	read_lock_irq(&tasklist_lock);
+	spin_lock(&runqueue_lock);
+
+	task->cpus_allowed = new_mask;
+
+	/*
+	 * if running on a different CPU, cause a reschedule
+	 * to move the process to an allowed CPU
+	 */
+	if (!(task->cpus_runnable & task->cpus_allowed)) {
+		if (task == current)
+			reschedule = 1;
+		else {
+			task->need_resched = 1;
+			smp_send_reschedule(task->processor);
+		}
+	}
+
+	spin_unlock(&runqueue_lock);
+	read_unlock_irq(&tasklist_lock);
+
+	/*
+	 * if the task is on this CPU, wait to reschedule
+	 * it until we drop the locks
+	 */
+	if (reschedule)
+		schedule();
+
+	return endp - buffer;
+}
+#endif /* CONFIG_SMP */

diff -urN linux-2.4.16/include/linux/capability.h linux/include/linux/capability.h
@@ -243,6 +243,7 @@
 /* Allow use of FIFO and round-robin (realtime) scheduling on own
    processes and setting the scheduling algorithm used by another
    process. */
+/* Allow setting CPU affinity */
 
 #define CAP_SYS_NICE         23
 

diff -urN linux-2.4.16/include/linux/proc_fs_i.h linux/include/linux/proc_fs_i.h
@@ -1,9 +1,11 @@
 struct proc_inode_info {
 	struct task_struct *task;
 	int type;
-	union {
+	struct {
 		int (*proc_get_link)(struct inode *, struct dentry **, struct vfsmount **);
 		int (*proc_read)(struct task_struct *task, char *page);
+		int (*proc_write)(struct task_struct *task, char *page,
+				  size_t bytes);
 	} op;
 	struct file *file;
 };

As I said in reply to Ingo patch, it'd be better to expose "number" cpu masks not "logical" ( like cpus_allowed ).
In this way the users can use 0..N-1 ( N == number of cpus phisically available ) w/out having to know the internal mapping between logical and number ids.

- Davide

Do you mean you don't like using a bitmask ?

00000001 = first CPU, its not logical, its physical.

Plus, how do you intend to set multiple non-contiguous CPUs? A fraction of them? With only a 32-bit value?

Note also that my patch understands the underlying CPU nature, such that "echo 0000ffff > /proc/123/affinity" will only affine task 123 to your first 2 CPUs if you only have two. Thus, "cat /proc/123/affinity" will return "00000003".

Robert Love

No no, I love binary math :)

If You set this directly to cpus_allowed You're going to set the logical one.
This because cpus_allowed is logical, because it's used like, ie:

int this_cpu = p->processor;

if (p->cpus_allowed & (1 << this_cpu)) ...

and p->processor is logical.
Something like :

unsigned long num2log_mask(unsigned long cmsk) {
	unsigned long msk = 0;
	int ii;

	for (ii = 0; ii < smp_num_cpus; ii++)
		if (cmsk & (1 << ii))
			msk |= (1 << cpu_logical_map(ii));
	return msk;
}

unsigned long log2num_mask(unsigned long cmsk) {
	unsigned long msk = 0;
	int ii;

	for (ii = 0; ii < smp_num_cpus; ii++)
		if (cmsk & (1 << ii))
			msk |= (1 << cpu_number_map(ii));
	return msk;
}

You use num2log_mask() on the user input mask to set cpus_allowed and log2num_mask() to return the map to the user.

- Davide

	for (ii = 0; ii < NR_CPUS; ii++)

- Davide

yep, agreed. I've uploaded a new set-affinity syscall patch with your improvement added:

	http://redhat.com/~mingo/set-affinity-patches/set-affinity-2.4.16-A0

i've only tested it on x86 which has a 1:1 mapping between physical and logical CPUs, but it should be fine on other architectures as well.

Ingo

The snippet I sent yesterday should be corrected by checking if cpu_{number,logical}_map(ii) is != -1 to avoid incorrect bit settings, expecially from mask coming from user side.

- Davide

indeed. New patch is at:

http://redhat.com/~mingo/set-affinity-patches/set-affinity-2.4.16-A1

(the buggy function was not a security problem, it just produces unexpected bitmaps.)

Ingo

Have you seen Andrew Mortons cpus_allowed patch?

http://www.zipworld.com.au/~akpm/linux/cpus_allowed.patch

Anton

No, although Andrew mentioned he had worked on it to me.

Looking it over...its of course very similar. One thing to note is his patch doesn't incorporate the bits from Ingo's patch that I borrowed. I.e., use of cpu_online_map, the forcing of the reschedule, etc. I favor some of the differences in my variant.

I am glad to see he wrote a proc_write function, because I was worried I was reimplementing something there, but I suppose not.

Either way, I'd like to see one of these in the kernel.

Robert Love

I really need to push pset - I guess I should finish the 2.4.x port, eh?

www.hockin.org/~thockin/pset

Hmm, now that I think about it, anyone should be able to restrict the CPUs that their processes should run on, but like "nice", you should have CAP_SYS_NICE in order to increase the number of CPUs your process can run on. This makes it possible to "throttle" a user so that they can only max out a single CPU.

Why would you do that? Maybe because "ulimit" and friends only allow you to set an absolute limit on the number of CPU seconds you can use per process, but not a "percentage" of a processor or some equivalent "cycles per second" unit.

Not that I see this as being hugely necessary, but it may as well be consistent with current behaviour (c.f. nice, ulimit, etc, can all go down, but not necessarily up).

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

I thought about this too, and I realized that cpus_allowed is by default 0xffffffff (i.e., all procs in the system). That means this is only an issue when the administrator explicitly changed affinity.

And that would occur in the situation you describe ... personally, I think users should be able to set their masks but I completely see your argument.

I suppose we could count the bits in cpus_allowed and make sure they are less than or equal to the new_mask bits. This seems overkill, though. We could just not allow users to change their own affinity without CAP_SYS_NICE, and allow root to do anything (via CAP_SYS_ADMIN).

Anyone else have an opinion?

Robert Love

two comments. First, this has already been done - Andrew Morton has written such a patch.

Second, as i've repeatedly said it, it's a failure to do this over /proc. What if /proc is not mounted? What if the process is in a chroot() environment, should it not be able to set its own affinity? This is a fundamental limitation of your approach, and *if* we want to export the cpus_allowed affinity to user-space (which is up to discussion), then the right way (TM) to do it is via a syscall.

Ingo

HP and others have already implemented chunks of this stuff via syscall interfaces. There is a complete pset api.

Alan

the sched_set_affinity() syscall tries to be simple, and uses the existing ->cpus_allowed mechanizm. Most of the pset patches i've seen so far are IMHO overdoing this issue a bit, interface-wise, and do not provide more than this simple solution.

Ingo

I didn't know this until after I started, but it is irrelevant. Use Andrew's if you want. However, I have incorporated some useful bits from your patch and such that I think are superior.

OK OK OK ... we can argue all day over syscall vs. proc. Personally, I don't find any of the arguments fruitful -- we make all sorts of restrictions and "Don't do thats" in the kernel. Requiring procfs isn't the end of the world.

When you posted your initial patch, I commented I liked it but was interested in a proc variant. Some people were interested. Even you said it wasn't a huge deal.

It doesn't matter to me, let's just expose _some_ interface to userspace. Personally, I prefer procfs, but both implementations are nicely done. I respect you too much to argue religion like this. I'll push for either variant.

Robert Love