All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] SMP signal latency fix up.
@ 2003-11-06 22:49 Mark Gross
  2003-11-06 23:20 ` Linus Torvalds
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Mark Gross @ 2003-11-06 22:49 UTC (permalink / raw)
  To: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

The attached program should execute the following command line within a fraction of a second.
time ./ping_pong -c 10000

Running on SMP and the 2.6.0-test9 kernel, it takes about 10000 * 1/HZ seconds.  Running this 
command with maxcpus=1 the command finishes in fraction of a second.  Under SMP the 
signal delivery isn't kicking the task if its in the run state on the other CPU.

The following patch has been tested and seems to fix the problem.  
I'm confident about the change to sched.c actualy fixes a cut and paste bug.

The change to signal.c IS needed to fix the problem, but I'm not sure there isn't a better way.

Please have take a look

--mgross

diff -urN -X dontdiff linux-2.6.0-test9/kernel/sched.c /opt/linux-2.6.0-test9/kernel/sched.c
--- linux-2.6.0-test9/kernel/sched.c    2003-10-25 11:44:29.000000000 -0700
+++ /opt/linux-2.6.0-test9/kernel/sched.c       2003-11-06 13:04:03.628116240 -0800
@@ -626,13 +626,13 @@
                        }
                        success = 1;
                }
-#ifdef CONFIG_SMP
-               else
-                       if (unlikely(kick) && task_running(rq, p) && (task_cpu(p) != smp_processor_id()))
-                               smp_send_reschedule(task_cpu(p));
-#endif
                p->state = TASK_RUNNING;
        }
+#ifdef CONFIG_SMP
+               else
+               if (unlikely(kick) && task_running(rq, p) && (task_cpu(p) != smp_processor_id()))
+                       smp_send_reschedule(task_cpu(p));
+#endif
        task_rq_unlock(rq, &flags);

        return success;
diff -urN -X dontdiff linux-2.6.0-test9/kernel/signal.c /opt/linux-2.6.0-test9/kernel/signal.c
--- linux-2.6.0-test9/kernel/signal.c   2003-10-25 11:43:27.000000000 -0700
+++ /opt/linux-2.6.0-test9/kernel/signal.c      2003-11-06 12:18:22.000000000 -0800
@@ -555,6 +555,9 @@
                wake_up_process_kick(t);
                return;
        }
+       if (t->state == TASK_RUNNING ) 
+               wake_up_process_kick(t);
+       
 }

 /*

[-- Attachment #2: ping_pong.c --]
[-- Type: text/x-c, Size: 3210 bytes --]

#include <argp.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <math.h>
#include <pthread.h>
#include <signal.h>

int argp_parse_option(int key, char *arg, struct argp_state *argp_state);

struct state {
	pthread_t receive;
	pthread_t send;
	int signal;
	int count;
	int yield;
        volatile int flags;
	volatile int sync_flags;
};

static struct state _state = {
	.signal = SIGUSR1,
	.flags = 0,
	.sync_flags = 0,
	.yield = 0,
};
static const struct argp_option argp_options[] = {
	{"count", 'c', "COUNT", 0, "Number of signals exchanged"},
	{"signal", 'n', "SIGNUM", 0, "The sended signal"},
	{"yield", 'y', NULL, 0, "Make receiver yield while waiting"},
	{0}
};
static const struct argp argp = {
	.options = argp_options,
	.parser = argp_parse_option,
};

int argp_parse_option(int key, char *arg, struct argp_state *argp_state)
{
	struct state *state = argp_state->input;
	switch (key) {		
	case 'n':
		if (sscanf(arg, "%u", &state->signal) != 1) {
			perror("invalid signal argument");
			return -EINVAL;
		}		
		break;
	case 'c':
		if (sscanf(arg, "%i", &state->count) != 1) {
			perror("invalid count argument");
			return -EINVAL;
		}		
		break;
	case 'y':
		state->yield = 1;
		break;
		
	}

	return 0;
}


static void * sender (void *arg)
{
	struct state * state = (struct state *) arg;
	sigset_t action;
	int c = 0, r = 0;
	int sig;
	
	sigemptyset (&action);
	sigaddset (&action, state->signal);
	sigprocmask (SIG_BLOCK, &action, 0);

	/* wait for the receiver to get ready for receiving signals */
	while (state->sync_flags == 0)
		sched_yield();

	while ( c++ < state->count ) {
		/* ping */
		pthread_kill ( state->receive, state->signal);	

		/* pong */
		if ((r = sigwait(&action, &sig)) != 0) {
			perror("sigwait");
			state->flags = 1;
			pthread_exit(&r);
		}
	}

	state->flags = 1;
 	pthread_exit (&r);	
}
   
static void handler(int signum)
{
	struct state * state = &_state;
	pthread_kill(state->send, state->signal);
}
static void * receiver (void *arg)
{
	struct state * state = (struct state *) arg;
	struct sigaction action;
	int r;
	
	action.sa_handler = handler;
	sigemptyset (&action.sa_mask);
	action.sa_flags = 0;	
	if ((r = sigaction (state->signal, &action, 0)) != 0) {
		perror("sigaction");
		pthread_exit(&r);
	}

	/* let the sender thread know we are ready to start receiving */
	state->sync_flags = 1;

	while ( state->flags == 0 ) { 
		if (state->yield)
			sched_yield();
		else
			/*spin*/;
	}

	/* ...and done */
	pthread_exit(&r);	
}	

int main(int argc, char *argv[])
{
	int result = 0;
	void * thread_result;
	struct state *state = &_state;

	if (argp_parse(&argp, argc, argv, ARGP_IN_ORDER, 0, state) != 0)
		exit(-1);

	result = pthread_create (&state->receive, NULL, receiver, state);
	if ( result != 0 ) {
		perror("pthread_create");
		exit(-1);
	}

	result = pthread_create (&state->send, NULL, sender, state);
	if ( result != 0 ) {
		perror("pthread_create");
		exit(-1);
	}

	result = pthread_join ( state->send, &thread_result);	
	if ( result != 0 ) {
		perror("pthread_join");
		exit(-1);
	}

	result = pthread_join ( state->receive, &thread_result);	
	if ( result != 0 ) {
		perror("pthread_join");
		exit(-1);
	}
	
	return 0;
}

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH] SMP signal latency fix up.
@ 2003-11-06 23:00 Mark Gross
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Gross @ 2003-11-06 23:00 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org


The program after the patch should execute the following command line within a fraction of a second.
time ./ping_pong -c 10000

Running on SMP and the 2.6.0-test9 kernel, it takes about 10000 * 1/HZ seconds.  Running this 
command with maxcpus=1 the command finishes in fraction of a second.  Under SMP the 
signal delivery isn't kicking the task if its in the run state on the other CPU.

The following patch has been tested and seems to fix the problem.  
I'm confident about the change to sched.c actualy fixes a cut and paste bug.

The change to signal.c IS needed to fix the problem, but I'm not sure there isn't a better way.

Please have take a look

--mgross

diff -urN -X dontdiff linux-2.6.0-test9/kernel/sched.c /opt/linux-2.6.0-test9/kernel/sched.c
--- linux-2.6.0-test9/kernel/sched.c    2003-10-25 11:44:29.000000000 -0700
+++ /opt/linux-2.6.0-test9/kernel/sched.c       2003-11-06 13:04:03.628116240 -0800
@@ -626,13 +626,13 @@
                        }
                        success = 1;
                }
-#ifdef CONFIG_SMP
-               else
-                       if (unlikely(kick) && task_running(rq, p) && (task_cpu(p) != smp_processor_id()))
-                               smp_send_reschedule(task_cpu(p));
-#endif
                p->state = TASK_RUNNING;
        }
+#ifdef CONFIG_SMP
+               else
+               if (unlikely(kick) && task_running(rq, p) && (task_cpu(p) != smp_processor_id()))
+                       smp_send_reschedule(task_cpu(p));
+#endif
        task_rq_unlock(rq, &flags);

        return success;
diff -urN -X dontdiff linux-2.6.0-test9/kernel/signal.c /opt/linux-2.6.0-test9/kernel/signal.c
--- linux-2.6.0-test9/kernel/signal.c   2003-10-25 11:43:27.000000000 -0700
+++ /opt/linux-2.6.0-test9/kernel/signal.c      2003-11-06 12:18:22.000000000 -0800
@@ -555,6 +555,9 @@
                wake_up_process_kick(t);
                return;
        }
+       if (t->state == TASK_RUNNING ) 
+               wake_up_process_kick(t);
+       
 }

 /*


---------------------------------- test program ---------------------------
/* ping_pong.c */

#include <argp.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <math.h>
#include <pthread.h>
#include <signal.h>

int argp_parse_option(int key, char *arg, struct argp_state *argp_state);

struct state {
	pthread_t receive;
	pthread_t send;
	int signal;
	int count;
	int yield;
        volatile int flags;
	volatile int sync_flags;
};

static struct state _state = {
	.signal = SIGUSR1,
	.flags = 0,
	.sync_flags = 0,
	.yield = 0,
};
static const struct argp_option argp_options[] = {
	{"count", 'c', "COUNT", 0, "Number of signals exchanged"},
	{"signal", 'n', "SIGNUM", 0, "The sended signal"},
	{"yield", 'y', NULL, 0, "Make receiver yield while waiting"},
	{0}
};
static const struct argp argp = {
	.options = argp_options,
	.parser = argp_parse_option,
};

int argp_parse_option(int key, char *arg, struct argp_state *argp_state)
{
	struct state *state = argp_state->input;
	switch (key) {		
	case 'n':
		if (sscanf(arg, "%u", &state->signal) != 1) {
			perror("invalid signal argument");
			return -EINVAL;
		}		
		break;
	case 'c':
		if (sscanf(arg, "%i", &state->count) != 1) {
			perror("invalid count argument");
			return -EINVAL;
		}		
		break;
	case 'y':
		state->yield = 1;
		break;
		
	}

	return 0;
}


static void * sender (void *arg)
{
	struct state * state = (struct state *) arg;
	sigset_t action;
	int c = 0, r = 0;
	int sig;
	
	sigemptyset (&action);
	sigaddset (&action, state->signal);
	sigprocmask (SIG_BLOCK, &action, 0);

	/* wait for the receiver to get ready for receiving signals */
	while (state->sync_flags == 0)
		sched_yield();

	while ( c++ < state->count ) {
		/* ping */
		pthread_kill ( state->receive, state->signal);	

		/* pong */
		if ((r = sigwait(&action, &sig)) != 0) {
			perror("sigwait");
			state->flags = 1;
			pthread_exit(&r);
		}
	}

	state->flags = 1;
 	pthread_exit (&r);	
}
   
static void handler(int signum)
{
	struct state * state = &_state;
	pthread_kill(state->send, state->signal);
}
static void * receiver (void *arg)
{
	struct state * state = (struct state *) arg;
	struct sigaction action;
	int r;
	
	action.sa_handler = handler;
	sigemptyset (&action.sa_mask);
	action.sa_flags = 0;	
	if ((r = sigaction (state->signal, &action, 0)) != 0) {
		perror("sigaction");
		pthread_exit(&r);
	}

	/* let the sender thread know we are ready to start receiving */
	state->sync_flags = 1;

	while ( state->flags == 0 ) { 
		if (state->yield)
			sched_yield();
		else
			/*spin*/;
	}

	/* ...and done */
	pthread_exit(&r);	
}	

int main(int argc, char *argv[])
{
	int result = 0;
	void * thread_result;
	struct state *state = &_state;

	if (argp_parse(&argp, argc, argv, ARGP_IN_ORDER, 0, state) != 0)
		exit(-1);

	result = pthread_create (&state->receive, NULL, receiver, state);
	if ( result != 0 ) {
		perror("pthread_create");
		exit(-1);
	}

	result = pthread_create (&state->send, NULL, sender, state);
	if ( result != 0 ) {
		perror("pthread_create");
		exit(-1);
	}

	result = pthread_join ( state->send, &thread_result);	
	if ( result != 0 ) {
		perror("pthread_join");
		exit(-1);
	}

	result = pthread_join ( state->receive, &thread_result);	
	if ( result != 0 ) {
		perror("pthread_join");
		exit(-1);
	}
	
	return 0;
}


^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2003-11-08  6:49 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-11-06 22:49 [PATCH] SMP signal latency fix up Mark Gross
2003-11-06 23:20 ` Linus Torvalds
2003-11-07  1:39   ` Mark Gross
2003-11-07  1:42     ` Mark Gross
2003-11-07  9:45       ` Ingo Molnar
2003-11-07 15:43         ` Mark Gross
2003-11-07  9:39   ` Ingo Molnar
2003-11-07 15:09     ` Linus Torvalds
2003-11-07 15:17       ` Ingo Molnar
2003-11-07 15:31         ` Ingo Molnar
2003-11-07 15:29       ` Ingo Molnar
2003-11-07 17:03     ` Mark Gross
2003-11-08  6:48       ` Ingo Molnar
2003-11-06 23:26 ` Chris Friesen
2003-11-06 23:35   ` Mark Gross
2003-11-07  0:55     ` Nuno Silva
2003-11-07 10:24 ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2003-11-06 23:00 Mark Gross

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.