[2.6 patch] fix sched_setscheduler semantics

[2.6 patch] fix sched_setscheduler semantics

[Jason Baron]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1357 bytes --]


hi,

Currently, a negative policy argument passed into the 
'sys_sched_setscheduler()' system call, will return with success.  
However, the manpage for 'sys_sched_setscheduler' says:

EINVAL The scheduling policy is not one of the recognized policies, or the
              parameter p does not make sense for the policy.

'sys_sched_setscheduler()' is implemented as a wrapper around 
'sched_setscheduler()', which treats negative values as 'use current 
policy'. This is used to implement sys_sched_setparam(), see:

asmlinkage long sys_sched_setparam(pid_t pid, struct sched_param __user 
*param)
{
        return do_sched_setscheduler(pid, -1, param);
}


Therefore, i'd suggest the following patch. Verified to fix the attached 
test case. Thanks to Tony Reix for pointing this out.

-Jason



Signed-off-by: Jason Baron <jbaron@redhat.com

 sched.c |    4 ++++
 1 files changed, 4 insertions(+)

--- linux-2.6/kernel/sched.c.bak	2006-01-16 15:55:16.000000000 -0500
+++ linux-2.6/kernel/sched.c	2006-01-16 15:56:23.000000000 -0500
@@ -3824,6 +3824,10 @@ do_sched_setscheduler(pid_t pid, int pol
 asmlinkage long sys_sched_setscheduler(pid_t pid, int policy,
 				       struct sched_param __user *param)
 {
+	/* negative values for policy are not valid */
+	if (policy < 0)
+		return -EINVAL;
+
 	return do_sched_setscheduler(pid, policy, param);
 }
 

[-- Attachment #2: setscheduler test --]
[-- Type: TEXT/PLAIN, Size: 2190 bytes --]

/* 
 *  This program is free software; you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License version 2.
 *
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *  GNU General Public License for more details.
 *
 *
 * Test that sched_setscheduler() sets errno == EINVAL when the policy value is
 * not defined in the sched.h header.
 *
 * Assume that the header does not defined a scheduling policy with a value
 * of -1. (It is more coherent with the specificationS of sched_getscheduler
 * and sched_setscheduler for which the result code -1 indicate an error.)
 * If no error occurs whith -1, the test will run sched_setscheduler with the
 * very improbable policy value INVALID_POLICY.
 */
#include <sched.h>
#include <stdio.h>
#include <errno.h>
#include <unistd.h>
#include "posixtest.h"


/* There is no chance that a scheduling policy has such a value */
#define INVALID_POLICY -27367

int main(){
	int result;
	struct sched_param param;

	param.sched_priority = 0;

	result = sched_setscheduler(0, -1, &param);

	if(result == -1 && errno == EINVAL){
		printf("Test PASSED\n");
		return PTS_PASS;
	} else if(errno == EPERM){
		printf("This process does not have the permission to set its own scheduling policy.\nTry to launch this test as root.\n");
		return PTS_UNRESOLVED;
	} else if(errno == 0) {
		printf("No error occurs, check if -1 a valid value for the scheduling policy.\n");
	} else {
		perror("Unknow error");
		return PTS_FAIL;
	}

	printf("Testing with very improbable policy value %i:\n",
	       INVALID_POLICY);

	result = sched_setscheduler(0, INVALID_POLICY, &param);

	if(result == -1 && errno == EINVAL){
		printf("Test PASSED with policy value %i\n", INVALID_POLICY);
		return PTS_PASS; 
	} else if(errno == 0) {
		printf("No error occurs, could %i be a valid value for the scheduling policy ???\n", INVALID_POLICY);
		return PTS_UNRESOLVED;
	} else {
		perror("Unknow error");
		return PTS_FAIL;
	}


}

Re: [2.6 patch] fix sched_setscheduler semantics

[Alexey Dobriyan]

On Mon, Jan 16, 2006 at 05:17:55PM -0500, Jason Baron wrote:
> --- linux-2.6/kernel/sched.c.bak
> +++ linux-2.6/kernel/sched.c
> @@ -3824,6 +3824,10 @@ do_sched_setscheduler(pid_t pid, int pol
>  asmlinkage long sys_sched_setscheduler(pid_t pid, int policy,
>  				       struct sched_param __user *param)
>  {
> +	/* negative values for policy are not valid */
> +	if (policy < 0)
> +		return -EINVAL;

Classical redundant comment.

Re: [2.6 patch] fix sched_setscheduler semantics

[Alan Cox]

On Maw, 2006-01-17 at 01:52 +0300, Alexey Dobriyan wrote:
> On Mon, Jan 16, 2006 at 05:17:55PM -0500, Jason Baron wrote:
> > --- linux-2.6/kernel/sched.c.bak
> > +++ linux-2.6/kernel/sched.c
> > @@ -3824,6 +3824,10 @@ do_sched_setscheduler(pid_t pid, int pol
> >  asmlinkage long sys_sched_setscheduler(pid_t pid, int policy,
> >  				       struct sched_param __user *param)
> >  {
> > +	/* negative values for policy are not valid */
> > +	if (policy < 0)
> > +		return -EINVAL;
> 
> Classical redundant comment.

Disagree. A pointless comment would be "if policy is negative return
-EINVAL". The comment makes it clear that policy < 0 is *invalid* as a
syscall argument rather than just something not currently handled, or
being done for algorithmic reasons

Re: [2.6 patch] fix sched_setscheduler semantics

[Ingo Molnar]

* Jason Baron <jbaron@redhat.com> wrote:

> Therefore, i'd suggest the following patch. Verified to fix the 
> attached test case. Thanks to Tony Reix for pointing this out.

indeed - good catch.

>  asmlinkage long sys_sched_setscheduler(pid_t pid, int policy,
>  				       struct sched_param __user *param)
>  {
> +	/* negative values for policy are not valid */
> +	if (policy < 0)
> +		return -EINVAL;
> +
>  	return do_sched_setscheduler(pid, policy, param);

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo