Cleanup. __down_common() can use the new signal_pending_state() helper.
Signed-off-by: Oleg Nesterov <[email protected]>
--- 26-rc2/kernel/semaphore.c~2_SEM_USE_HELPER 2008-05-18 15:44:19.000000000 +0400
+++ 26-rc2/kernel/semaphore.c 2008-06-04 20:45:21.000000000 +0400
@@ -211,9 +211,7 @@ static inline int __sched __down_common(
waiter.up = 0;
for (;;) {
- if (state == TASK_INTERRUPTIBLE && signal_pending(task))
- goto interrupted;
- if (state == TASK_KILLABLE && fatal_signal_pending(task))
+ if (signal_pending_state(state, task))
goto interrupted;
if (timeout <= 0)
goto timed_out;
On Wed, Jun 04, 2008 at 09:09:06PM +0400, Oleg Nesterov wrote:
> Cleanup. __down_common() can use the new signal_pending_state() helper.
This is a bad optimisation. __down_common gets inlined and the constant
'state' versions are optimised away for the versions which don't apply.
NAK this patch.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On 06/04, Matthew Wilcox wrote:
>
> On Wed, Jun 04, 2008 at 09:09:06PM +0400, Oleg Nesterov wrote:
> > Cleanup. __down_common() can use the new signal_pending_state() helper.
>
> This is a bad optimisation. __down_common gets inlined and the constant
> 'state' versions are optimised away for the versions which don't apply.
>
> NAK this patch.
OK.
But this was not optimisation, just code re-use.
Actually, I don't understand why __down_common/__mutex_lock_common are inlines,
perhaps it is better to shrink .text instead.
For example, "size kernel/mutex.o" reports
2715 0 12 2727 aa7 kernel/mutex.o
with __mutex_lock_common() uninlined, we have
1565 0 12 1577 629 kernel/mutex.o
the difference is more than K.
Oleg.
On Wed, Jun 04, 2008 at 10:08:58PM +0400, Oleg Nesterov wrote:
> Actually, I don't understand why __down_common/__mutex_lock_common are inlines,
> perhaps it is better to shrink .text instead.
I believe Ingo did extensive benchmarking to determine that the current
situation with mutex_lock_common is the best one. I didn't, I assumed
that following Ingo's lead would be the best idea.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
* Matthew Wilcox <[email protected]> wrote:
> On Wed, Jun 04, 2008 at 09:09:06PM +0400, Oleg Nesterov wrote:
> > Cleanup. __down_common() can use the new signal_pending_state() helper.
>
> This is a bad optimisation. __down_common gets inlined and the
> constant 'state' versions are optimised away for the versions which
> don't apply.
>
> NAK this patch.
well but we had a schedule() bug around this whole topic of
TASK_KILLABLE, so i'd say robustness trumps micro-optimizations. Is this
really a big issue? The dual-ness of TASK_KILLABLE checks is a bit ugly,
and it led to a bug as well.
Ingo