introduce wait_event_common()
Subsystems (5)
v2 cover introduce wait_event_common()
Hello.
The changes in include/linux/wait.h are the same.
However 1/2 has to update the callers of __wait_event_interruptible(). The fix is very simple:
- __wait_event_interruptible(wq, cond, ret); + ret = __wait_event_interruptible(wq, cond);
but obviously untested.
Oleg.
arch/mips/kernel/rtlx.c | 17 ++-- include/linux/wait.h | 183 +++++++++++++++------------------------ kernel/wait.c | 13 +++ net/irda/af_irda.c | 5 +- net/netfilter/ipvs/ip_vs_sync.c | 5 +- 5 files changed, 95 insertions(+), 128 deletions(-)
PATCH v2 1/2 wait: introduce wait_event_common(wq, condition, state, timeout)
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.
--- 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(-)
arch/mips/kernel/rtlx.c +8 -9
@@ -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; }
include/linux/wait.h +70 -111
234 lines not on this page · raw message
net/irda/af_irda.c +2 -3
@@ -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));
net/netfilter/ipvs/ip_vs_sync.c +1 -4
@@ -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; }
Signature · 1 line
1.5.5.1
PATCH v2 2/2 wait: introduce prepare_to_wait_event()
In reply to Oleg NesterovAdd 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.
--- include/linux/wait.h | 22 +++++++++++----------- kernel/wait.c | 13 +++++++++++++ 2 files changed, 24 insertions(+), 11 deletions(-)
include/linux/wait.h +11 -11
@@ -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,
kernel/wait.c +13 -0
@@ -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) {
Signature · 1 line
1.5.5.1