[PATCH] add wait_event_interruptible_exclusive() macro

[PATCH] add wait_event_interruptible_exclusive() macro

[Dean Nelson]

This patch defines a macro that does exactly what wait_event_interruptible()
does except that it adds the current task to the wait queue as an exclusive
task (i.e., sets the WQ_FLAG_EXCLUSIVE flag) rather than as a non-exclusive
task as wait_event_interruptible() does.

This allows one to do a wake_up_nr() to wake up a specific number of tasks.
I'm in the process of submitting a patch to linux-ia64 that requires this
capability. (Its subject line is "[PATCH 3/4] SGI Altix cross partition
functionality".)


Index: linux/include/linux/wait.h
===================================================================
--- linux.orig/include/linux/wait.h
+++ linux/include/linux/wait.h
@@ -200,7 +200,36 @@
 		__wait_event_interruptible_timeout(wq, condition, __ret); \
 	__ret;								\
 })
-	
+
+#define __wait_event_interruptible_exclusive(wq, condition, ret)	\
+do {									\
+	wait_queue_t __wait;						\
+	init_waitqueue_entry(&__wait, current);				\
+									\
+	add_wait_queue_exclusive(&wq, &__wait);				\
+	for (;;) {							\
+		set_current_state(TASK_INTERRUPTIBLE);			\
+		if (condition)						\
+			break;						\
+		if (!signal_pending(current)) {				\
+			schedule();					\
+			continue;					\
+		}							\
+		ret = -ERESTARTSYS;					\
+		break;							\
+	}								\
+	current->state = TASK_RUNNING;					\
+	remove_wait_queue(&wq, &__wait);				\
+} while (0)
+
+#define wait_event_interruptible_exclusive(wq, condition)		\
+({									\
+	int __ret = 0;							\
+	if (!(condition))						\
+		__wait_event_interruptible_exclusive(wq, condition, __ret);\
+	__ret;								\
+})
+
 /*
  * Must be called with the spinlock in the wait_queue_head_t held.
  */

Re: [PATCH] add wait_event_interruptible_exclusive() macro

[Chris Wedgwood]

On Fri, Jun 18, 2004 at 10:12:06AM -0500, Dean Nelson wrote:

> +#define __wait_event_interruptible_exclusive(wq, condition, ret)	\
> +do {									\
> +	wait_queue_t __wait;						\
> +	init_waitqueue_entry(&__wait, current);				\
> +									\
> +	add_wait_queue_exclusive(&wq, &__wait);
> \

[...]

Thsi reminds me...

I really loath all the preprocessor macros.  I know there are plenty
of this already, but I don't see the advantage of macros over (static)
inline functions which IMO look cleaner and give gcc some change to
sanitize what it's looking at without actually having to have it used.

Is there a reason why we keep doing this?



   --cw

Re: [PATCH] add wait_event_interruptible_exclusive() macro

[Dean Nelson]

On Tue, Jun 22, 2004 at 05:01:30AM -0700, Chris Wedgwood wrote:
> On Fri, Jun 18, 2004 at 10:12:06AM -0500, Dean Nelson wrote:
> 
> > +#define __wait_event_interruptible_exclusive(wq, condition, ret)	\
> > +do {									\
> > +	wait_queue_t __wait;						\
> > +	init_waitqueue_entry(&__wait, current);				\
> > +									\
> > +	add_wait_queue_exclusive(&wq, &__wait);
> > \
> 
> [...]
> 
> Thsi reminds me...
> 
> I really loath all the preprocessor macros.  I know there are plenty
> of this already, but I don't see the advantage of macros over (static)
> inline functions which IMO look cleaner and give gcc some change to
> sanitize what it's looking at without actually having to have it used.
> 
> Is there a reason why we keep doing this?

In this particular case, I was just trying to 'conform' to what I found
in include/linux/wait.h.

If the community would prefer, I can resubmit my patch as an inline
function instead of a macro.

Dean

Re: [PATCH] add wait_event_interruptible_exclusive() macro

[Christoph Hellwig]

On Tue, Jun 22, 2004 at 05:01:30AM -0700, Chris Wedgwood wrote:
> Thsi reminds me...
> 
> I really loath all the preprocessor macros.  I know there are plenty
> of this already, but I don't see the advantage of macros over (static)
> inline functions which IMO look cleaner and give gcc some change to
> sanitize what it's looking at without actually having to have it used.
> 
> Is there a reason why we keep doing this?

In this case a macro is the only sensible way.  Check how the arguments
are used in wait_event_*

Re: [PATCH] add wait_event_interruptible_exclusive() macro

[Paul Jackson]

Christoph wrote:
> In this case a macro is the only sensible way.  Check how the arguments
> are used in wait_event_*

Are you referring to such usages as the "&wq" in this line:

>	add_wait_queue_exclusive(&wq, &__wait);

If so, then yes, a macro is needed.  This is a poor man's implicit pass
by reference.  In straight C, not using macros, saying "foo(x)" pushes
the value of 'x' on the stack, not the address of 'x'.  This macro
pushes the _address_ of 'x' on the stack (x == wq), but doesn't require
the "address of" operator, "&", as in foo(&x), to be explicitly coded. 
Hence, straight C is insufficient to the task.

I don't know if it applies in this case, but one can mix macros with
inlines, to get both this implicit pass by reference, and get some
type checking on the arguments.  See my dreaded cpumask patch, in the
latest *-mm patch series, for many examples.

They look like:

#define foo(x) __foo(&(x))

static inline void __foo(int *xp)		\
{						\
	printk("addr of x is %p\n", xp);	\
}

This both provides implicit pass by reference, and checks that the
argument and return types are as intended (to within the range of
C's automatic type conversions ;).

Be aware that I am engaging in drive-by-commenting here, which is
commenting on a phrase that jumped out at me, without any effort to
look around at the larger picture.  Damage due to collateral fire is
not uncommon in such cases ;).

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: [PATCH] add wait_event_interruptible_exclusive() macro

[Christoph Hellwig]

On Tue, Jun 22, 2004 at 06:35:37AM -0700, Paul Jackson wrote:
> Are you referring to such usages as the "&wq" in this line:

No.  The real issue is how we use the condition argument.  in most users
that's no a simple variable but a complex statatement.