On Tue, Oct 01, 2013 at 07:01:37PM +0200, Oleg Nesterov wrote:
> This patch moves the signal-pending checks and part of DEFINE_WAIT's
> code into the new helper: prepare_to_wait_event().
>
> Yes, sure, prepare_to_wait_event() becomes a little bit slower than
> prepare_to_wait/prepare_to_wait_exclusive. But this is the slow path
> anyway, we are likely going to sleep. IMO, it is better to shrink
> .text, and on my build the difference is
>
> - 5124686 2955056 10117120 18196862 115a97e vmlinux
> + 5123212 2955088 10117120 18195420 115a3dc vmlinux
>
> The code with the patch is
>
> #define ___wait_is_interruptible(state)
> \
> (!__builtin_constant_p(state) ||
> \
> state == TASK_INTERRUPTIBLE || state == TASK_KILLABLE)
> \
>
> #define ___wait_event(wq, condition, state, exclusive, ret, cmd)
> \
> ({
> \
> __label__ __out;
> \
> wait_queue_t __wait;
> \
> long __ret = ret;
> \
>
> \
> INIT_LIST_HEAD(&__wait.task_list);
> \
> if (exclusive)
> \
> __wait.flags = WQ_FLAG_EXCLUSIVE;
> \
> else
> \
> __wait.flags = 0;
> \
__wait.flags = exclusive * WQ_FLAG_EXCLUSIVE;
or is that too obscure? ;-)
>
> \
> for (;;) {
> \
> long intr = prepare_to_wait_event(&wq, &__wait, state);
> \
int __intr = ...;
The interruptible bit doesn't actually need long; and local variables
have __ prefixes in this context.
>
> \
> if (condition)
> \
> break;
> \
>
> \
> if (___wait_is_interruptible(state) && intr) {
> \
> __ret = intr;
> \
> if (exclusive) {
> \
> abort_exclusive_wait(&wq, &__wait,
> \
> state, NULL);
> \
> goto __out;
> \
> }
> \
> break;
> \
> }
> \
>
> \
> cmd;
> \
> }
> \
> finish_wait(&wq, &__wait);
> \
> __out: __ret;
> \
> })
>
> Compiler should optimize out "long intr" if !interruptible/killable.
Yeah, and I think even the if (0 && __intr) would suffice for the unused
check; otherwise we'd have to adorn the thing with __maybe_unused.
> What do you think?
That would actually work I think.. the ___wait_is_interruptible() nicely
does away with the unused code; the only slightly more expensive thing
would be the prepare_to_wait_event() thing.
And if that really turns out to be a problem we could even re-use
___wait_is_interruptible() to call prepare_to_wait() instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
