* [PATCH v2 1/2] wait: introduce wait_event_common(wq, condition, state, timeout)
2013-06-24 15:43 [PATCH v2 0/2] introduce wait_event_common() Oleg Nesterov
@ 2013-06-24 15:44 ` Oleg Nesterov
2013-06-24 15:44 ` [PATCH v2 2/2] wait: introduce prepare_to_wait_event() Oleg Nesterov
1 sibling, 0 replies; 3+ messages in thread
From: Oleg Nesterov @ 2013-06-24 15:44 UTC (permalink / raw)
To: Andrew Morton
Cc: Daniel Vetter, Imre Deak, Julian Anastasov, Lukas Czerner,
Ralf Baechle, Samuel Ortiz, Simon Horman, Tejun Heo,
Wensong Zhang, linux-kernel
1. 4c663cfc "fix false timeouts when using wait_event_timeout()"
is not enough, wait(wq, true, 0) still returns zero.
__wait_event_timeout() was already fixed but we need the same
logic in wait_event_timeout() if the fast-path check succeeds.
2. wait_event_timeout/__wait_event_timeout interface do not match
wait_event(), you can't use __wait_event_timeout() instead of
wait_event_timeout() if you do not need the fast-path check.
Same for wait_event_interruptible/__wait_event_interruptible,
so this patch cleanups rtlx.c, ip_vs_sync.c, and af_irda.c:
- __wait_event_interruptible(wq, cond, ret);
+ ret = __wait_event_interruptible(wq, cond);
3. wait_event_* macros duplicate the same code.
This patch adds a single helper wait_event_common() which hopefully
does everything right. Compiler optimizes out the "dead" code when
we do not need signal_pending/schedule_timeout.
"size vmlinux" reports:
text data bss dec hex filename
- 4978601 2935080 10104832 18018513 112f0d1 vmlinux
+ 4977769 2930984 10104832 18013585 112dd91 vmlinux
but I think this depends on gcc/config.
In particular, wait_even_timeout(true, non_const_timeout) should
generate more code in the non-void context because the patch adds
the additional code to fix the 1st problem.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Tejun Heo <tj@kernel.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Lukas Czerner <lczerner@redhat.com>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: Wensong Zhang <wensong@linux-vs.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
arch/mips/kernel/rtlx.c | 17 ++--
include/linux/wait.h | 181 +++++++++++++++------------------------
net/irda/af_irda.c | 5 +-
net/netfilter/ipvs/ip_vs_sync.c | 5 +-
4 files changed, 81 insertions(+), 127 deletions(-)
diff --git a/arch/mips/kernel/rtlx.c b/arch/mips/kernel/rtlx.c
index 6fa198d..5f11ff6 100644
--- a/arch/mips/kernel/rtlx.c
+++ b/arch/mips/kernel/rtlx.c
@@ -172,8 +172,8 @@ int rtlx_open(int index, int can_sleep)
if (rtlx == NULL) {
if( (p = vpe_get_shared(tclimit)) == NULL) {
if (can_sleep) {
- __wait_event_interruptible(channel_wqs[index].lx_queue,
- (p = vpe_get_shared(tclimit)), ret);
+ ret = __wait_event_interruptible(channel_wqs[index].lx_queue,
+ (p = vpe_get_shared(tclimit)));
if (ret)
goto out_fail;
} else {
@@ -263,11 +263,11 @@ unsigned int rtlx_read_poll(int index, int can_sleep)
/* data available to read? */
if (chan->lx_read == chan->lx_write) {
if (can_sleep) {
- int ret = 0;
+ int ret;
- __wait_event_interruptible(channel_wqs[index].lx_queue,
+ ret = __wait_event_interruptible(channel_wqs[index].lx_queue,
(chan->lx_read != chan->lx_write) ||
- sp_stopping, ret);
+ sp_stopping);
if (ret)
return ret;
@@ -441,14 +441,13 @@ static ssize_t file_write(struct file *file, const char __user * buffer,
/* any space left... */
if (!rtlx_write_poll(minor)) {
- int ret = 0;
+ int ret;
if (file->f_flags & O_NONBLOCK)
return -EAGAIN;
- __wait_event_interruptible(channel_wqs[minor].rt_queue,
- rtlx_write_poll(minor),
- ret);
+ ret = __wait_event_interruptible(channel_wqs[minor].rt_queue,
+ rtlx_write_poll(minor));
if (ret)
return ret;
}
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 1133695..6b78045 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -173,18 +173,52 @@ wait_queue_head_t *bit_waitqueue(void *, int);
#define wake_up_interruptible_sync_poll(x, m) \
__wake_up_sync_key((x), TASK_INTERRUPTIBLE, 1, (void *) (m))
-#define __wait_event(wq, condition) \
-do { \
+#define __wait_no_timeout(tout) \
+ (__builtin_constant_p(tout) && (tout) == MAX_SCHEDULE_TIMEOUT)
+
+/* uglified signal_pending_state() optimized for constant state */
+#define __wait_signal_pending(state) \
+ ((state == TASK_INTERRUPTIBLE) ? signal_pending(current) : \
+ (state == TASK_KILLABLE) ? fatal_signal_pending(current) : \
+ 0)
+
+#define __wait_event_common(wq, condition, state, tout) \
+({ \
DEFINE_WAIT(__wait); \
+ long __ret = 0, __tout = tout; \
\
for (;;) { \
- prepare_to_wait(&wq, &__wait, TASK_UNINTERRUPTIBLE); \
- if (condition) \
+ prepare_to_wait(&wq, &__wait, state); \
+ if (condition) { \
+ __ret = __wait_no_timeout(tout) ?: __tout ?: 1; \
+ break; \
+ } \
+ \
+ if (__wait_signal_pending(state)) { \
+ __ret = -ERESTARTSYS; \
+ break; \
+ } \
+ \
+ if (__wait_no_timeout(tout)) \
+ schedule(); \
+ else if (__tout) \
+ __tout = schedule_timeout(__tout); \
+ else \
break; \
- schedule(); \
} \
finish_wait(&wq, &__wait); \
-} while (0)
+ __ret; \
+})
+
+#define wait_event_common(wq, condition, state, tout) \
+({ \
+ long __ret; \
+ if (condition) \
+ __ret = __wait_no_timeout(tout) ?: (tout) ?: 1; \
+ else \
+ __ret = __wait_event_common(wq, condition, state, tout);\
+ __ret; \
+})
/**
* wait_event - sleep until a condition gets true
@@ -198,29 +232,13 @@ do { \
* wake_up() has to be called after changing any variable that could
* change the result of the wait condition.
*/
-#define wait_event(wq, condition) \
-do { \
- if (condition) \
- break; \
- __wait_event(wq, condition); \
-} while (0)
+#define __wait_event(wq, condition) \
+ __wait_event_common(wq, condition, \
+ TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT) \
-#define __wait_event_timeout(wq, condition, ret) \
-do { \
- DEFINE_WAIT(__wait); \
- \
- for (;;) { \
- prepare_to_wait(&wq, &__wait, TASK_UNINTERRUPTIBLE); \
- if (condition) \
- break; \
- ret = schedule_timeout(ret); \
- if (!ret) \
- break; \
- } \
- if (!ret && (condition)) \
- ret = 1; \
- finish_wait(&wq, &__wait); \
-} while (0)
+#define wait_event(wq, condition) \
+ wait_event_common(wq, condition, \
+ TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT) \
/**
* wait_event_timeout - sleep until a condition gets true or a timeout elapses
@@ -239,31 +257,13 @@ do { \
* jiffies (at least 1) if the @condition evaluated to %true before
* the @timeout elapsed.
*/
-#define wait_event_timeout(wq, condition, timeout) \
-({ \
- long __ret = timeout; \
- if (!(condition)) \
- __wait_event_timeout(wq, condition, __ret); \
- __ret; \
-})
+#define __wait_event_timeout(wq, condition, timeout) \
+ __wait_event_common(wq, condition, \
+ TASK_UNINTERRUPTIBLE, timeout)
-#define __wait_event_interruptible(wq, condition, ret) \
-do { \
- DEFINE_WAIT(__wait); \
- \
- for (;;) { \
- prepare_to_wait(&wq, &__wait, TASK_INTERRUPTIBLE); \
- if (condition) \
- break; \
- if (!signal_pending(current)) { \
- schedule(); \
- continue; \
- } \
- ret = -ERESTARTSYS; \
- break; \
- } \
- finish_wait(&wq, &__wait); \
-} while (0)
+#define wait_event_timeout(wq, condition, timeout) \
+ wait_event_common(wq, condition, \
+ TASK_UNINTERRUPTIBLE, timeout)
/**
* wait_event_interruptible - sleep until a condition gets true
@@ -280,35 +280,13 @@ do { \
* The function will return -ERESTARTSYS if it was interrupted by a
* signal and 0 if @condition evaluated to true.
*/
-#define wait_event_interruptible(wq, condition) \
-({ \
- int __ret = 0; \
- if (!(condition)) \
- __wait_event_interruptible(wq, condition, __ret); \
- __ret; \
-})
+#define __wait_event_interruptible(wq, condition) \
+ __wait_event_common(wq, condition, \
+ TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT)
-#define __wait_event_interruptible_timeout(wq, condition, ret) \
-do { \
- DEFINE_WAIT(__wait); \
- \
- for (;;) { \
- prepare_to_wait(&wq, &__wait, TASK_INTERRUPTIBLE); \
- if (condition) \
- break; \
- if (!signal_pending(current)) { \
- ret = schedule_timeout(ret); \
- if (!ret) \
- break; \
- continue; \
- } \
- ret = -ERESTARTSYS; \
- break; \
- } \
- if (!ret && (condition)) \
- ret = 1; \
- finish_wait(&wq, &__wait); \
-} while (0)
+#define wait_event_interruptible(wq, condition) \
+ wait_event_common(wq, condition, \
+ TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT)
/**
* wait_event_interruptible_timeout - sleep until a condition gets true or a timeout elapses
@@ -328,13 +306,13 @@ do { \
* a signal, or the remaining jiffies (at least 1) if the @condition
* evaluated to %true before the @timeout elapsed.
*/
+#define __wait_event_interruptible_timeout(wq, condition, timeout) \
+ __wait_event_common(wq, condition, \
+ TASK_INTERRUPTIBLE, timeout)
+
#define wait_event_interruptible_timeout(wq, condition, timeout) \
-({ \
- long __ret = timeout; \
- if (!(condition)) \
- __wait_event_interruptible_timeout(wq, condition, __ret); \
- __ret; \
-})
+ wait_event_common(wq, condition, \
+ TASK_INTERRUPTIBLE, timeout)
#define __wait_event_hrtimeout(wq, condition, timeout, state) \
({ \
@@ -601,24 +579,6 @@ do { \
-#define __wait_event_killable(wq, condition, ret) \
-do { \
- DEFINE_WAIT(__wait); \
- \
- for (;;) { \
- prepare_to_wait(&wq, &__wait, TASK_KILLABLE); \
- if (condition) \
- break; \
- if (!fatal_signal_pending(current)) { \
- schedule(); \
- continue; \
- } \
- ret = -ERESTARTSYS; \
- break; \
- } \
- finish_wait(&wq, &__wait); \
-} while (0)
-
/**
* wait_event_killable - sleep until a condition gets true
* @wq: the waitqueue to wait on
@@ -634,14 +594,13 @@ do { \
* The function will return -ERESTARTSYS if it was interrupted by a
* signal and 0 if @condition evaluated to true.
*/
-#define wait_event_killable(wq, condition) \
-({ \
- int __ret = 0; \
- if (!(condition)) \
- __wait_event_killable(wq, condition, __ret); \
- __ret; \
-})
+#define __wait_event_killable(wq, condition) \
+ __wait_event_common(wq, condition, \
+ TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT)
+#define wait_event_killable(wq, condition) \
+ wait_event_common(wq, condition, \
+ TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT)
#define __wait_event_lock_irq(wq, condition, lock, cmd) \
do { \
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 0578d4f..0f67690 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -2563,9 +2563,8 @@ bed:
jiffies + msecs_to_jiffies(val));
/* Wait for IR-LMP to call us back */
- __wait_event_interruptible(self->query_wait,
- (self->cachedaddr != 0 || self->errno == -ETIME),
- err);
+ err = __wait_event_interruptible(self->query_wait,
+ (self->cachedaddr != 0 || self->errno == -ETIME));
/* If watchdog is still activated, kill it! */
del_timer(&(self->watchdog));
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index f6046d9..ce4239e 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1624,12 +1624,9 @@ static int sync_thread_master(void *data)
continue;
}
while (ip_vs_send_sync_msg(tinfo->sock, sb->mesg) < 0) {
- int ret = 0;
-
__wait_event_interruptible(*sk_sleep(sk),
sock_writeable(sk) ||
- kthread_should_stop(),
- ret);
+ kthread_should_stop());
if (unlikely(kthread_should_stop()))
goto done;
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v2 2/2] wait: introduce prepare_to_wait_event()
2013-06-24 15:43 [PATCH v2 0/2] introduce wait_event_common() Oleg Nesterov
2013-06-24 15:44 ` [PATCH v2 1/2] wait: introduce wait_event_common(wq, condition, state, timeout) Oleg Nesterov
@ 2013-06-24 15:44 ` Oleg Nesterov
1 sibling, 0 replies; 3+ messages in thread
From: Oleg Nesterov @ 2013-06-24 15:44 UTC (permalink / raw)
To: Andrew Morton
Cc: Daniel Vetter, Imre Deak, Julian Anastasov, Lukas Czerner,
Ralf Baechle, Samuel Ortiz, Simon Horman, Tejun Heo,
Wensong Zhang, linux-kernel
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,
- 4977769 2930984 10104832 18013585 112dd91 vmlinux
+ 4976847 2930984 10104832 18012663 112d9f7 vmlinux
on my build.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Tejun Heo <tj@kernel.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Lukas Czerner <lczerner@redhat.com>
---
include/linux/wait.h | 22 +++++++++++-----------
kernel/wait.c | 13 +++++++++++++
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 6b78045..6e0fe54 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -176,28 +176,27 @@ wait_queue_head_t *bit_waitqueue(void *, int);
#define __wait_no_timeout(tout) \
(__builtin_constant_p(tout) && (tout) == MAX_SCHEDULE_TIMEOUT)
-/* uglified signal_pending_state() optimized for constant state */
-#define __wait_signal_pending(state) \
- ((state == TASK_INTERRUPTIBLE) ? signal_pending(current) : \
- (state == TASK_KILLABLE) ? fatal_signal_pending(current) : \
- 0)
+#define __wait_interruptible(state) \
+ (!__builtin_constant_p(state) || \
+ state == TASK_INTERRUPTIBLE || state == TASK_KILLABLE)
#define __wait_event_common(wq, condition, state, tout) \
({ \
- DEFINE_WAIT(__wait); \
- long __ret = 0, __tout = tout; \
+ long __ret, __tout = tout; \
+ wait_queue_t __wait; \
+ \
+ INIT_LIST_HEAD(&__wait.task_list); \
+ __wait.flags = 0; \
\
for (;;) { \
- prepare_to_wait(&wq, &__wait, state); \
+ __ret = prepare_to_wait_event(&wq, &__wait, state); \
if (condition) { \
__ret = __wait_no_timeout(tout) ?: __tout ?: 1; \
break; \
} \
\
- if (__wait_signal_pending(state)) { \
- __ret = -ERESTARTSYS; \
+ if (__wait_interruptible(state) && __ret) \
break; \
- } \
\
if (__wait_no_timeout(tout)) \
schedule(); \
@@ -781,6 +780,7 @@ extern long interruptible_sleep_on_timeout(wait_queue_head_t *q,
* Waitqueues which are removed from the waitqueue_head at wakeup time
*/
void prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state);
+int prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state);
void prepare_to_wait_exclusive(wait_queue_head_t *q, wait_queue_t *wait, int state);
void finish_wait(wait_queue_head_t *q, wait_queue_t *wait);
void abort_exclusive_wait(wait_queue_head_t *q, wait_queue_t *wait,
diff --git a/kernel/wait.c b/kernel/wait.c
index 6698e0c..3b8619a 100644
--- a/kernel/wait.c
+++ b/kernel/wait.c
@@ -78,6 +78,19 @@ prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state)
}
EXPORT_SYMBOL(prepare_to_wait);
+int 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);
+
+ return 0;
+}
+EXPORT_SYMBOL(prepare_to_wait_event);
+
void
prepare_to_wait_exclusive(wait_queue_head_t *q, wait_queue_t *wait, int state)
{
--
1.5.5.1
^ permalink raw reply related [flat|nested] 3+ messages in thread