2003-11-06 22:51:47

by Mark Gross

[permalink] [raw]
Subject: [PATCH] SMP signal latency fix up.

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);
+
}

/*


Attachments:
ping_pong.c (3.13 kB)

2003-11-06 23:26:50

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] SMP signal latency fix up.


On 6 Nov 2003, Mark Gross wrote:
>
> 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.

It looks like the "wake_up_process_kick()" interface is just broken. As it
stands now, it's literally _designed_ to kick the process only when it
wakes it up, which is silly and wrong. It makes no sense to kick a process
that we just woke up, because it _will_ react immediately anyway.

We literally want to kick only processes that didn't need waking up, and
the current interface is totally unsuitable for that.

> 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.

Naah, it's a thinko, the code shouldn't be like that at all.

There's only one user of the "wake_up_process_kick()" thing, and that one
user really wants to kick the process totally independently of waking it
up. Which just implies that we should just have a _regular_
"wake_up_process()" there, and a _separate_ "kick_process()" thing.

So I've got a feeling that
- we should remove the "kick" argument from "try_to_wake_up()"
- the signal wakeup case should instead do a _regular_ wakeup.
- we should kick the process if the wakeup _fails_.

Ie signal wakeup should most likely look something like


inline void signal_wake_up(struct task_struct *t, int resume)
{
int woken;
unsigned int mask;

set_tsk_thread_flag(t,TIF_SIGPENDING);
mask = TASK_INTERRUPTIBLE;
if (resume)
mask |= TASK_STOPPED;
woken = 0;
if (t->state & mask)
woken = wake_up_state(p, mask);
if (!woken)
kick_process(p);
}

where the "kick_process()" thing does the "is the task running on some
other CPU and if so send it a reschedule event to make it react" thing.

Ingo?

Linus

> 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);
> +
> }
>
> /*
>

2003-11-07 01:41:29

by Mark Gross

[permalink] [raw]
Subject: Re: [PATCH] SMP signal latency fix up.

On Thu, 2003-11-06 at 15:20, Linus Torvalds wrote:
> On 6 Nov 2003, Mark Gross wrote:
> >
> > 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.
>
> It looks like the "wake_up_process_kick()" interface is just broken. As it
> stands now, it's literally _designed_ to kick the process only when it
> wakes it up, which is silly and wrong. It makes no sense to kick a process
> that we just woke up, because it _will_ react immediately anyway.
>
> We literally want to kick only processes that didn't need waking up, and
> the current interface is totally unsuitable for that.
>
> > 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.
>
> Naah, it's a thinko, the code shouldn't be like that at all.
>
> There's only one user of the "wake_up_process_kick()" thing, and that one
> user really wants to kick the process totally independently of waking it
> up. Which just implies that we should just have a _regular_
> "wake_up_process()" there, and a _separate_ "kick_process()" thing.
>
> So I've got a feeling that
> - we should remove the "kick" argument from "try_to_wake_up()"
> - the signal wakeup case should instead do a _regular_ wakeup.
> - we should kick the process if the wakeup _fails_.
>
> Ie signal wakeup should most likely look something like
>
>
> inline void signal_wake_up(struct task_struct *t, int resume)
> {
> int woken;
> unsigned int mask;
>
> set_tsk_thread_flag(t,TIF_SIGPENDING);
> mask = TASK_INTERRUPTIBLE;
> if (resume)
> mask |= TASK_STOPPED;
> woken = 0;
> if (t->state & mask)
> woken = wake_up_state(p, mask);
> if (!woken)
> kick_process(p);
> }
>
> where the "kick_process()" thing does the "is the task running on some
> other CPU and if so send it a reschedule event to make it react" thing.
>
> Ingo?
>
> Linus


Are you thinking about something like this?

It seems to work. I dropped the "task_running" test from
smp_process_kick intentionaly. As well as the movement of the success
flag. I hope its not too wrong.

It seems to work on a HT P4 desktop and a dual PIII box.

--mgross

diff -urN -X dontdiff linux-2.6.0-test9/include/linux/sched.h
/opt/linux-2.6.0-test9/include/linux/sched.h
--- linux-2.6.0-test9/include/linux/sched.h 2003-10-25
11:42:56.000000000 -0700
+++ /opt/linux-2.6.0-test9/include/linux/sched.h 2003-11-06
14:44:22.000000000 -0800
@@ -573,7 +573,7 @@

extern int FASTCALL(wake_up_state(struct task_struct * tsk, unsigned
int state));
extern int FASTCALL(wake_up_process(struct task_struct * tsk));
-extern int FASTCALL(wake_up_process_kick(struct task_struct * tsk));
+extern void FASTCALL(smp_process_kick(struct task_struct * tsk));
extern void FASTCALL(wake_up_forked_process(struct task_struct * tsk));
extern void FASTCALL(sched_exit(task_t * p));

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 14:58:29.000000000
-0800
@@ -585,7 +585,7 @@
*
* returns failure only if the task is already active.
*/
-static int try_to_wake_up(task_t * p, unsigned int state, int sync, int
kick)
+static int try_to_wake_up(task_t * p, unsigned int state, int sync)
{
unsigned long flags;
int success = 0;
@@ -624,13 +624,8 @@
if (TASK_PREEMPTS_CURR(p, rq))
resched_task(rq->curr);
}
- 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
+ success = 1;
p->state = TASK_RUNNING;
}
task_rq_unlock(rq, &flags);
@@ -640,19 +635,22 @@

int wake_up_process(task_t * p)
{
- return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE |
TASK_UNINTERRUPTIBLE, 0, 0);
+ return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE |
TASK_UNINTERRUPTIBLE, 0);
}

EXPORT_SYMBOL(wake_up_process);

-int wake_up_process_kick(task_t * p)
+void smp_process_kick(task_t * p)
{
- return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE |
TASK_UNINTERRUPTIBLE, 0, 1);
+#ifdef CONFIG_SMP
+ if (task_cpu(p) != smp_processor_id())
+ smp_send_reschedule(task_cpu(p));
+#endif
}

int wake_up_state(task_t *p, unsigned int state)
{
- return try_to_wake_up(p, state, 0, 0);
+ return try_to_wake_up(p, state, 0);
}

/*
@@ -1624,7 +1622,7 @@
int default_wake_function(wait_queue_t *curr, unsigned mode, int sync)
{
task_t *p = curr->task;
- return try_to_wake_up(p, mode, sync, 0);
+ return try_to_wake_up(p, mode, sync);
}

EXPORT_SYMBOL(default_wake_function);
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 15:05:41.945237624
-0800
@@ -538,6 +538,7 @@
inline void signal_wake_up(struct task_struct *t, int resume)
{
unsigned int mask;
+ int woken;

set_tsk_thread_flag(t,TIF_SIGPENDING);

@@ -551,10 +552,14 @@
mask = TASK_INTERRUPTIBLE;
if (resume)
mask |= TASK_STOPPED;
+ woken = 0;
if (t->state & mask) {
- wake_up_process_kick(t);
- return;
+ woken = wake_up_process(t);
}
+#ifdef CONFIG_SMP
+ if (!woken)
+ smp_process_kick(t);
+#endif
}

/*




2003-11-07 22:25:53

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] SMP signal latency fix up.


On Thu, 6 Nov 2003, Mark Gross wrote:

> 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.

yeah - good catch - it's a brown paper bag bug.

Ingo

2003-11-07 22:21:57

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] SMP signal latency fix up.


On Thu, 6 Nov 2003, Linus Torvalds wrote:

> So I've got a feeling that
> - we should remove the "kick" argument from "try_to_wake_up()"
> - the signal wakeup case should instead do a _regular_ wakeup.
> - we should kick the process if the wakeup _fails_.

agreed - and this essential mechanism was my intention when i added the
kick argument originally. The problem is solved the simplest way via the
patch below - ie. i missed the other !success branch within
try_to_wake_up().

--- linux/kernel/sched.c.orig
+++ linux/kernel/sched.c
@@ -626,13 +626,12 @@ repeat_lock_task:
}
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
+ if (unlikely(kick) && !success && task_running(rq, p) && (task_cpu(p) != smp_processor_id()))
+ smp_send_reschedule(task_cpu(p));
+#endif
task_rq_unlock(rq, &flags);

return success;

(note that this is slightly different from Mark's patch.)

but i fully agree with your other suggestion - there's no problem with
sending the IPI later and outside of the wakeup spinlock. In fact doing so
removes a variable from the wakeup hotpath and cleans up stuff. Hence i'd
suggest to apply the attached patch which implements your suggestion. I've
tested it and it solves the latency problem of the code Mark posted. It
compiles & boots on both UP and SMP x86.

Ingo

--- linux/include/linux/sched.h.orig
+++ linux/include/linux/sched.h
@@ -574,7 +574,11 @@ extern void do_timer(struct pt_regs *);

extern int FASTCALL(wake_up_state(struct task_struct * tsk, unsigned int state));
extern int FASTCALL(wake_up_process(struct task_struct * tsk));
-extern int FASTCALL(wake_up_process_kick(struct task_struct * tsk));
+#ifdef CONFIG_SMP
+ extern void FASTCALL(kick_process(struct task_struct * tsk));
+#else
+ static inline void kick_process(struct task_struct *tsk) { }
+#endif
extern void FASTCALL(wake_up_forked_process(struct task_struct * tsk));
extern void FASTCALL(sched_exit(task_t * p));

--- linux/kernel/sched.c.orig
+++ linux/kernel/sched.c
@@ -530,6 +530,15 @@ static inline void resched_task(task_t *
#endif
}

+/**
+ * task_curr - is this task currently executing on a CPU?
+ * @p: the task in question.
+ */
+inline int task_curr(task_t *p)
+{
+ return cpu_curr(task_cpu(p)) == p;
+}
+
#ifdef CONFIG_SMP

/*
@@ -568,6 +577,27 @@ repeat:
task_rq_unlock(rq, &flags);
preempt_enable();
}
+
+/***
+ * kick_process - kick a running thread to enter/exit the kernel
+ * @p: the to-be-kicked thread
+ *
+ * Cause a process which is running on another CPU to enter
+ * kernel-mode, without any delay. (to get signals handled.)
+ */
+void kick_process(task_t *p)
+{
+ int cpu;
+
+ preempt_disable();
+ cpu = task_cpu(p);
+ if ((cpu != smp_processor_id()) && task_curr(p))
+ smp_send_reschedule(cpu);
+ preempt_enable();
+}
+
+EXPORT_SYMBOL_GPL(kick_process);
+
#endif

/***
@@ -575,7 +605,6 @@ repeat:
* @p: the to-be-woken-up thread
* @state: the mask of task states that can be woken
* @sync: do a synchronous wakeup?
- * @kick: kick the CPU if the task is already running?
*
* Put it on the run-queue if it's not already there. The "current"
* thread is always on the run-queue (except when the actual
@@ -585,7 +614,7 @@ repeat:
*
* returns failure only if the task is already active.
*/
-static int try_to_wake_up(task_t * p, unsigned int state, int sync, int kick)
+static int try_to_wake_up(task_t * p, unsigned int state, int sync)
{
unsigned long flags;
int success = 0;
@@ -626,33 +655,22 @@ repeat_lock_task:
}
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;
}
task_rq_unlock(rq, &flags);

return success;
}
-
int wake_up_process(task_t * p)
{
- return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0, 0);
+ return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0);
}

EXPORT_SYMBOL(wake_up_process);

-int wake_up_process_kick(task_t * p)
-{
- return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0, 1);
-}
-
int wake_up_state(task_t *p, unsigned int state)
{
- return try_to_wake_up(p, state, 0, 0);
+ return try_to_wake_up(p, state, 0);
}

/*
@@ -1621,7 +1639,7 @@ EXPORT_SYMBOL(preempt_schedule);
int default_wake_function(wait_queue_t *curr, unsigned mode, int sync)
{
task_t *p = curr->task;
- return try_to_wake_up(p, mode, sync, 0);
+ return try_to_wake_up(p, mode, sync);
}

EXPORT_SYMBOL(default_wake_function);
@@ -1942,15 +1960,6 @@ int task_nice(task_t *p)
EXPORT_SYMBOL(task_nice);

/**
- * task_curr - is this task currently executing on a CPU?
- * @p: the task in question.
- */
-int task_curr(task_t *p)
-{
- return cpu_curr(task_cpu(p)) == p;
-}
-
-/**
* idle_cpu - is a given cpu idle currently?
* @cpu: the processor in question.
*/
--- linux/kernel/signal.c.orig
+++ linux/kernel/signal.c
@@ -538,8 +538,9 @@ int dequeue_signal(struct task_struct *t
inline void signal_wake_up(struct task_struct *t, int resume)
{
unsigned int mask;
+ int woken;

- set_tsk_thread_flag(t,TIF_SIGPENDING);
+ set_tsk_thread_flag(t, TIF_SIGPENDING);

/*
* If resume is set, we want to wake it up in the TASK_STOPPED case.
@@ -551,10 +552,11 @@ inline void signal_wake_up(struct task_s
mask = TASK_INTERRUPTIBLE;
if (resume)
mask |= TASK_STOPPED;
- if (t->state & mask) {
- wake_up_process_kick(t);
- return;
- }
+ woken = 0;
+ if (t->state & mask)
+ woken = wake_up_state(t, mask);
+ if (!woken)
+ kick_process(t);
}

/*

2003-11-07 22:22:34

by Mark Gross

[permalink] [raw]
Subject: Re: [PATCH] SMP signal latency fix up.


This patch is very close to mine. It fixes that bug I had with my
change to success, and it adds a test to check if the kick target is
current.

Its hard for me to tell if its better to being more careful with
throughing around the IPI's at the cost of the extra opperations within
your kick code.

Looks correct, and works good too! I just verified that it solves my
signal latency issue on both my HT system and my dual PIII box. Where I
first found the problem.

FWIW I would like to see your patch go into 2.6.

Thanks!


On Fri, 2003-11-07 at 01:39, Ingo Molnar wrote:

> but i fully agree with your other suggestion - there's no problem with
> sending the IPI later and outside of the wakeup spinlock. In fact doing so
> removes a variable from the wakeup hotpath and cleans up stuff. Hence i'd
> suggest to apply the attached patch which implements your suggestion. I've
> tested it and it solves the latency problem of the code Mark posted. It
> compiles & boots on both UP and SMP x86.
>
> Ingo
>
> --- linux/include/linux/sched.h.orig
> +++ linux/include/linux/sched.h
> @@ -574,7 +574,11 @@ extern void do_timer(struct pt_regs *);
>
> extern int FASTCALL(wake_up_state(struct task_struct * tsk, unsigned int state));
> extern int FASTCALL(wake_up_process(struct task_struct * tsk));
> -extern int FASTCALL(wake_up_process_kick(struct task_struct * tsk));
> +#ifdef CONFIG_SMP
> + extern void FASTCALL(kick_process(struct task_struct * tsk));
> +#else
> + static inline void kick_process(struct task_struct *tsk) { }
> +#endif
> extern void FASTCALL(wake_up_forked_process(struct task_struct * tsk));
> extern void FASTCALL(sched_exit(task_t * p));
>
> --- linux/kernel/sched.c.orig
> +++ linux/kernel/sched.c
> @@ -530,6 +530,15 @@ static inline void resched_task(task_t *
> #endif
> }
>
> +/**
> + * task_curr - is this task currently executing on a CPU?
> + * @p: the task in question.
> + */
> +inline int task_curr(task_t *p)
> +{
> + return cpu_curr(task_cpu(p)) == p;
> +}
> +
> #ifdef CONFIG_SMP
>
> /*
> @@ -568,6 +577,27 @@ repeat:
> task_rq_unlock(rq, &flags);
> preempt_enable();
> }
> +
> +/***
> + * kick_process - kick a running thread to enter/exit the kernel
> + * @p: the to-be-kicked thread
> + *
> + * Cause a process which is running on another CPU to enter
> + * kernel-mode, without any delay. (to get signals handled.)
> + */
> +void kick_process(task_t *p)
> +{
> + int cpu;
> +
> + preempt_disable();
> + cpu = task_cpu(p);
> + if ((cpu != smp_processor_id()) && task_curr(p))
> + smp_send_reschedule(cpu);
> + preempt_enable();
> +}
> +
> +EXPORT_SYMBOL_GPL(kick_process);
> +
> #endif
>
> /***
> @@ -575,7 +605,6 @@ repeat:
> * @p: the to-be-woken-up thread
> * @state: the mask of task states that can be woken
> * @sync: do a synchronous wakeup?
> - * @kick: kick the CPU if the task is already running?
> *
> * Put it on the run-queue if it's not already there. The "current"
> * thread is always on the run-queue (except when the actual
> @@ -585,7 +614,7 @@ repeat:
> *
> * returns failure only if the task is already active.
> */
> -static int try_to_wake_up(task_t * p, unsigned int state, int sync, int kick)
> +static int try_to_wake_up(task_t * p, unsigned int state, int sync)
> {
> unsigned long flags;
> int success = 0;
> @@ -626,33 +655,22 @@ repeat_lock_task:
> }
> 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;
> }
> task_rq_unlock(rq, &flags);
>
> return success;
> }
> -
> int wake_up_process(task_t * p)
> {
> - return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0, 0);
> + return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0);
> }
>
> EXPORT_SYMBOL(wake_up_process);
>
> -int wake_up_process_kick(task_t * p)
> -{
> - return try_to_wake_up(p, TASK_STOPPED | TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0, 1);
> -}
> -
> int wake_up_state(task_t *p, unsigned int state)
> {
> - return try_to_wake_up(p, state, 0, 0);
> + return try_to_wake_up(p, state, 0);
> }
>
> /*
> @@ -1621,7 +1639,7 @@ EXPORT_SYMBOL(preempt_schedule);
> int default_wake_function(wait_queue_t *curr, unsigned mode, int sync)
> {
> task_t *p = curr->task;
> - return try_to_wake_up(p, mode, sync, 0);
> + return try_to_wake_up(p, mode, sync);
> }
>
> EXPORT_SYMBOL(default_wake_function);
> @@ -1942,15 +1960,6 @@ int task_nice(task_t *p)
> EXPORT_SYMBOL(task_nice);
>
> /**
> - * task_curr - is this task currently executing on a CPU?
> - * @p: the task in question.
> - */
> -int task_curr(task_t *p)
> -{
> - return cpu_curr(task_cpu(p)) == p;
> -}
> -
> -/**
> * idle_cpu - is a given cpu idle currently?
> * @cpu: the processor in question.
> */
> --- linux/kernel/signal.c.orig
> +++ linux/kernel/signal.c
> @@ -538,8 +538,9 @@ int dequeue_signal(struct task_struct *t
> inline void signal_wake_up(struct task_struct *t, int resume)
> {
> unsigned int mask;
> + int woken;
>
> - set_tsk_thread_flag(t,TIF_SIGPENDING);
> + set_tsk_thread_flag(t, TIF_SIGPENDING);
>
> /*
> * If resume is set, we want to wake it up in the TASK_STOPPED case.
> @@ -551,10 +552,11 @@ inline void signal_wake_up(struct task_s
> mask = TASK_INTERRUPTIBLE;
> if (resume)
> mask |= TASK_STOPPED;
> - if (t->state & mask) {
> - wake_up_process_kick(t);
> - return;
> - }
> + woken = 0;
> + if (t->state & mask)
> + woken = wake_up_state(t, mask);
> + if (!woken)
> + kick_process(t);
> }
>
> /*

2003-11-08 06:49:16

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] SMP signal latency fix up.


On Fri, 7 Nov 2003, Mark Gross wrote:

> Its hard for me to tell if its better to being more careful with
> throughing around the IPI's at the cost of the extra opperations within
> your kick code.

an IPI creates quite some overhead both on the source and on the target
CPU, so i think it's definitely worth this extra check. Also, with
increasingly higher load it's increasingly more likely that we can skip
the IPI (because the task might be on the runqueue but it is not
executing), so further increasing the load via additional IPIs is the
wrong answer.

> Looks correct, and works good too! I just verified that it solves my
> signal latency issue on both my HT system and my dual PIII box. Where I
> first found the problem.

great!

Ingo