linux-kernel - Re: [PATCH 2/2] wait: introduce prepare_to_wait

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 6 Jun 2013 14:36:36 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	Dave Jones <davej@...hat.com>,
	David Howells <dhowells@...hat.com>,
	Imre Deak <imre.deak@...el.com>, Jens Axboe <axboe@...nel.dk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Lukas Czerner <lczerner@...hat.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [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@...hat.com>

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

Reviewed-by: Tejun Heo <tj@...nel.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/