introduce wait_event_common()

PATCH Superseded Oleg Nesterov 3 participants 7 messages Jun 6, 2013 – Jun 19, 2013

Subsystems (2)

Networking Scheduler

lore

Share & tools

lore

Oleg Nesterov

author

2013-06-06 20:08 UTC #1 lore raw

PATCH 1/2 wait: introduce wait_event_common(wq, condition, state, timeout)

1. wait_event_timeout(wq, true, 0) returns zero, I think this is wrong and should be fixed.

__wait_event_timeout() was already changed by 4c663cfc 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.

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.

With this patch "size vmlinux" reports that .text/data shrinks but I think this depends on gcc/config.

Signed-off-by Oleg Nesterov <[email protected]>

Reviewed-by Tejun Heo <[email protected]>

---
 include/linux/wait.h |  181 +++++++++++++++++++------------------------------
 1 files changed, 70 insertions(+), 111 deletions(-)

include/linux/wait.h +70 ~~-111~~

index 1133695..392c54d 100644
@@ -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 {									\

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2013-06-06 20:08 UTC #4 lore raw

PATCH 2/2 wait: introduce prepare_to_wait_event()

In reply to Oleg Nesterov

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 <[email protected]>

---
 include/linux/wait.h |   22 +++++++++++-----------
 kernel/wait.c        |   14 ++++++++++++++
 2 files changed, 25 insertions(+), 11 deletions(-)

include/linux/wait.h +11 ~~-11~~

index 392c54d..a2314dd 100644
@@ -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 +14 -0

index 6698e0c..57cc6e7 100644
@@ -78,6 +78,20 @@ 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

Tejun Heo

Reviewed-by on v1 2/2

2013-06-06 21:36 UTC #5 lore raw

Re: [PATCH 2/2] wait: introduce prepare_to_wait_event()

Hello, Oleg.

Quoted 7 lines · Oleg Nesterov

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_*.

Quoted 8 lines

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 <[email protected]>

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

Reviewed-by Tejun Heo <[email protected]>

But a couple nits.

Quoted 1 line

+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. :)

kernel/wait.c prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state) · line 89

Quoted 8 lines

kernel/wait.c +8 -0

+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.

Signature · 1 line

tejun

Oleg Nesterov

author Reviewed-by Tejun Heo

2013-06-07 13:11 UTC #6 lore raw

Re: [PATCH 2/2] wait: introduce prepare_to_wait_event()

Hi Tejun,

Quoted 3 lines · Tejun Heo

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

Hmm. linus#master. I just verified that the patches apply cleanly after git-pull.

Quoted 1 line

Reviewed-by Tejun Heo <[email protected]>

Thanks,

Quoted 7 lines

But a couple nits.
Quoted 1 line
+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. :)

Yes, I tried to follow the style around the new helper, but the extra line is not needed.

Please see v2 below, no other changes.

Quoted 5 lines

(quoted diff) +5 -0

+prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state)
+{
+	if (signal_pending_state(state, current))
+		return -ERESTARTSYS;
+

Quoted 8 lines

Review
Quoted 3 lines
(quoted diff) +3 -0
+	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.

Not sure... As you can see from this patch, I like -Os more than -O2 ;)

------------------------------------------------------------------------------
Subject: [PATCH v2 2/2] wait: introduce prepare_to_wait_event()

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 <[email protected]>

Reviewed-by Tejun Heo <[email protected]>

---
 include/linux/wait.h |   22 +++++++++++-----------
 kernel/wait.c        |   13 +++++++++++++
 2 files changed, 24 insertions(+), 11 deletions(-)

include/linux/wait.h +11 ~~-11~~

index 392c54d..a2314dd 100644
@@ -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

index 6698e0c..3b8619a 100644
@@ -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

Andrew Morton

2013-06-18 22:06 UTC #2 lore raw

Re: [PATCH 1/2] wait: introduce wait_event_common(wq, condition, state, timeout)

In reply to Oleg Nesterov

Quoted 11 lines · On Thu, 6 Jun 2013 22:03:16 +0200 Oleg Nesterov <[email protected]> wrote:

1. wait_event_timeout(wq, true, 0) returns zero, I think this is wrong and should be fixed.
__wait_event_timeout() was already changed by 4c663cfc 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.

Quoted 8 lines

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.
With this patch "size vmlinux" reports that .text/data shrinks but I think this depends on gcc/config.

hm,

Review

Quoted 2 lines

(quoted diff) +1 -1

-#define __wait_event_interruptible(wq, condition, ret)
+#define __wait_event_interruptible(wq, condition)

net/irda/af_irda.c:2568:13: error: macro "__wait_event_interruptible" passed 3 arguments, but takes just 2

waddup with that?

__wait_event_interruptible() has several callsites. I think I'll go zap and await v2 ;)

Oleg Nesterov

author

2013-06-19 16:20 UTC #3 lore raw

Re: [PATCH 1/2] wait: introduce wait_event_common(wq, condition, state, timeout)

Quoted 6 lines · Andrew Morton

Review
Quoted 2 lines · On Thu, 6 Jun 2013 22:03:16 +0200 Oleg Nesterov <[email protected]> wrote:
(quoted diff) +1 -1
-#define __wait_event_interruptible(wq, condition, ret)
+#define __wait_event_interruptible(wq, condition)
net/irda/af_irda.c:2568:13: error: macro "__wait_event_interruptible" passed 3 arguments, but takes just 2

Aaah. Thanks.

Quoted 1 line

waddup with that?

Will read "man grep" again.

Quoted 2 lines

__wait_event_interruptible() has several callsites. I think I'll go zap and await v2 ;)

Yes, will do, thanks.

And note that one of the reasons for this patch was to make wait_event_interruptible() and __wait_event_interruptible() interchangeable, like wait_event() and __wait_event(). irda_getsockopt() uses __wait* exactly because it has already checked the condition.

Samuel, will you agree with the patch below?

Yes, this adds the unnecessary check. We will remove it later, after we change wait_event* macros, just

- err = wait_event_interruptible(...);
+ err = __wait_event_interruptible(...);

Otherwise, this patch should also change af_irda.c (and probably other callers), not good.

Oleg.

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));