On 06/05, Oleg Nesterov wrote:
>
> I think that wait_eveint_timeout(wq, COND, 0) should return !!(COND).
> But it doesn't, for example wait_event_timeout(wq, true, 0) == 0, this
> doesn'tlook right to me.
>
> And, this is off-topic, but wait_eveint_timeout/__wait_eveint_timeout
> do not match wait_event/__wait_event. IOW, you can't use
> __wait_eveint_timeout() if you do not need the fast-path check.
>
> So. How about
>
> #define __wait_event_timeout(wq, condition, timeout)
> \
> ({
> \
> DEFINE_WAIT(__wait);
> \
> long __ret = 0, __to = timeout;
> \
>
> \
> for (;;) {
> \
> prepare_to_wait(&wq, &__wait, TASK_UNINTERRUPTIBLE);
> \
> if (condition) {
> \
> __ret = __to ?: 1;
> \
> break;
> \
> }
> \
> if (!__to)
> \
> break;
> \
> __to = schedule_timeout(__to);
> \
> }
> \
> finish_wait(&wq, &__wait);
> \
> __ret;
> \
> })
>
> #define wait_event_timeout(wq, condition, timeout)
> \
> ({
> \
> long __ret;
> \
> if (condition)
> \
> __ret = (timeout) ?: 1;
> \
> else
> \
> __ret = __wait_event_timeout(wq, condition, timeout);
> \
> __ret;
> \
> })
>
> ?
>
> Othwerwise we should document the fact that the caller should alvays verify
> timeout != 0 if it checks the result of wait_event_timeout().
And in fact, perhaps we can implement wait_event_common() and avoid the
code duplications?
#define __wait_no_timeout(timeout) \
(__builtin_constant_p(timeout) && (timeout) ==
MAX_SCHEDULE_TIMEOUT)
/* uglified signal_pending_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, 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;
\
}
\
finish_wait(&wq, &__wait);
\
__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;
\
})
Then, for example, wait_event() becomes
#define wait_event(wq, condition)
\
wait_event_common(wq, condition,
\
TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT)
\
Hmm. I compiled the kernel with the patch below,
$ size vmlinux
text data bss dec hex filename
- 4978601 2935080 10104832 18018513 112f0d1 vmlinux
+ 4977769 2930984 10104832 18013585 112dd91 vmlinux
looks good...
Oleg.
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 1133695..359fffc 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(timeout) \
+ (__builtin_constant_p(timeout) && (timeout) == MAX_SCHEDULE_TIMEOUT)
+
+/* uglified signal_pending_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 { \
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
