lkml.org 
[lkml]   [2013]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] wait: introduce prepare_to_wait_event()
Hello, Oleg.

On Thu, Jun 06, 2013 at 10:03:34PM +0200, Oleg Nesterov wrote:
> Add the new helper, prepare_to_wait_event() which should only be used
> by wait_event_common/etc.
>
> prepare_to_wait_event() returns -ERESTARTSYS if signal_pending_state()
> is true, otherwise it calls prepare_to_wait(). This allows to uninline
> the signal-pending checks in wait_event_*.
>
> Also, it can initialize wait->private/func. We do not care they were
> already initialized, the values are the same. This also shaves a couple
> of insns from the inlined code.
>
> Unlike the previous change, this patch "reliably" shrinks the size of
> generated code for every wait_event*() call.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Which tree are the patches based on? I'm getting conflicts on
linus#master, mmotd and next.

Reviewed-by: Tejun Heo <tj@kernel.org>

But a couple nits.

> +int

I don't think we need to keep the unnecessary line break here. All
other functions in the file don't do it except for the two
prepare_to_wait functions. No need to give the weirdos more power. :)

> +prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state)
> +{
> + if (signal_pending_state(state, current))
> + return -ERESTARTSYS;
> +
> + wait->private = current;
> + wait->func = autoremove_wake_function;
> + prepare_to_wait(q, wait, state);

I wonder whether it'd be worthwhile to make prepare_to_wait() inline
so that it can be inlined into the above. I think gcc is smart enough
to emit inline for in-file stuff and then build a proper function for
external references. No biggie. Just wondering.

Thanks.

--
tejun


\
 
 \ /
  Last update: 2013-06-07 00:21    [W:0.057 / U:32.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site