Re: [RFC][PATCH] signals: don't copy siginfo_t on dequeue

[Vegard Nossum]

2009/2/26 Ingo Molnar <mingo@xxxxxxx>:
>
> * Vegard Nossum <vegard.nossum@xxxxxxxxx> wrote:
>
>> >From 60fc9a464377159ab807aec63277d4970019d631 Mon Sep 17 00:00:00 2001
>> From: Vegard Nossum <vegard.nossum@xxxxxxxxx>
>> Date: Thu, 26 Feb 2009 19:17:58 +0100
>> Subject: [PATCH] signals: don't copy siginfo_t on dequeue
>>
>> Instead of copying the siginfo_t whenever a signal is dequeued, just
>> get the pointer to the struct sigqueue, which can be freed by the
>> caller when the signal has been delivered.
>>
>> We can save kernel text (x86, 32-bit):
>>
>> $ scripts/bloat-o-meter vmlinux-unpatched vmlinux
>> add/remove: 0/0 grow/shrink: 3/7 up/down: 81/-538 (-457)
>> function                   old   new  delta
>> get_signal_to_deliver            Â871   922   +51
>> release_console_sem             Â459   481   +22
>> generate_resume_trace            Â611   619   Â+8
>> send_sigqueue                Â257   253   Â-4
>> vma_adjust                 Â1101  Â1093   Â-8
>> sys_rt_sigtimedwait             Â548   531   -17
>> dequeue_signal                415   372   -43
>> __dequeue_signal               388   259  Â-129
>> signalfd_read                1290  Â1139  Â-151
>> do_notify_resume              Â2216  Â2030  Â-186
>>
>> And we reduce stack pressure; In handle_signal() (in x86 code), we
>> replace a siginfo_t (128 bytes) with a pointer (8 bytes on x86_64),
>> and the same in signalfd_read().
>>
>> There is a slight slowdown (2.02% relative increase in CPU time):
>>
>>        unpatched    patched
>> ----------------------------------------
>> mean: Â Â Â Â 3.078500 Â Â Â Â3.140800
>> stddev: Â Â Â 0.074624 Â Â Â Â0.168989
>>
>> (Numbers are: CPU time in seconds, for two processes to
>> ping-pong in total 655360 SIGUSR1/SIGUSR2 signals between each
>> other. This was repeated 100 times for each kernel.)
>
> hm, does this SIGUSR1/SIGUSR2 test actually make use siginfo?

The delivery of a signal requires one copy_siginfo(). Likewise for
e.g. sys_kill(), which also requires one copy_siginfo(). So with this
patch, we have saved only the copy_siginfo() of the delivery path.

> I.e. shouldnt we have seen a speedup, due to not having to copy
> the siginfo structure?

Actually, no. Because copy_siginfo() does not copy the whole (128
byte) structure if the signal was generated by the user, just probably
the first 24 bytes or so. So I would expect the "kernel generated
signal" path to be faster, but that is not tested by my SIGUSR test.

Besides, I think there is a small overhead in now having an extra
level of indirection for getting our hands on the signal number.

But we might able to speed up the user->user case a little bit by
fixing sys_kill() not to use copy_siginfo() too.


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/