[KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix common

[KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix common

[Nishanth Aravamudan]

On 12.08.2006 [14:26:14 +1000], Darren Jenkins wrote:
> G'day
> 
> On 8/10/06, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > On Thu, Aug 10, 2006 at 08:48:19AM +1000, Darren Jenkins wrote:
> > > On 8/8/06, Pavel Machek <pavel@ucw.cz> wrote:
> > > > > Sorry I did not realise that was your problem with the code.
> > > > > Would it help if we just explicitly added a
> > > > >
> > > > > if (ready())
> > > > >        return 0;
> > > > >
> > > > > after the loop, in the example code? so people wont miss adding
> > > > > something like that in?
> > > >
> > > > Yes, that would do the trick.
> > > >                                                                 Pavel
> > >
> > >
> > > I was going to make a patch for the TODO, but it seems this subject
> > > has since been removed.
> > > Any chance it is going to be reinstated ? or is this subject
> > > considered not so much janitorial work?
> >
> > I need to read through those threads so correct version will be present
> > in TODO. So far, testing for OK condition was forgotten. Anything else?
> 
> No the problem that Pavel Machek showed us was the only one I saw.
> 
> >         unsigned long timeout = jiffies + HZ/2;
> >        for (;;) {
> >                if (ready())
> >                        return 0;
> > [IMAGINE HALF A SECOND DELAY HERE]
> >                if (time_after(timeout, jiffies))
> >                        break;
> >                msleep(10);
> >        }
> >
> > Oops
> 
> 
> 
> There were three proposed solutions though.

<snip>

FWIW, here is my poll_event() patch from some time ago updated to
2.6.18-rc4. I think it's closest in spirit to 3). Comments, thoughts?

Description: Add the poll_event(), poll_event_timeout(),
poll_event_interruptible() and poll_event_interruptible_timeout()
interfaces. These macros encapsulate typical sleeping code usage in a
sane way.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

diff -urpN 2.6.18-rc4/include/linux/delay.h 2.6.18-rc4-dev/include/linux/delay.h
--- 2.6.18-rc4/include/linux/delay.h	2006-08-11 22:40:57.000000000 -0700
+++ 2.6.18-rc4-dev/include/linux/delay.h	2006-08-11 23:26:23.000000000 -0700
@@ -44,4 +44,99 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+/*
+ * poll_event* check if a @condition is true every @interval milliseconds,
+ * up to a @timeout maximum, if specified, else forever.
+ * The *_interruptible versions will sleep in TASK_INTERRUPTIBLE
+ * (instead of TASK_UNINTERRUPTIBLE) and will return early on signals
+ * These interfaces are intentionally very similar to wait_event*
+ */
+
+/**
+ * poll_event - check a condition at regular intervals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ */
+#define poll_event(condition, interval)				\
+do {								\
+	while (!(condition))					\
+		msleep(interval);				\
+} while(0)
+
+/**
+ * poll_event_timeout - check a condition at regular intervals with a timeout
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * @timeout: Time in jiffies to stop checking at
+ * returns: 0, if condition caused return
+ * 	    -ETIME, if timeout
+ */
+#define poll_event_timeout(condition, interval, timeout)		\
+({									\
+	int __ret = 0;							\
+ 									\
+	for (;;) {							\
+		if (condition)						\
+			break;						\
+ 		msleep(interval);					\
+		if (time_after(jiffies, timeout)) {			\
+			__ret = -ETIME;					\
+ 			break;						\
+		}							\
+	}								\
+	__ret;								\
+})
+
+/**
+ * poll_event_interruptible - check a condition at regular intervals, returning early on signals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * returns: 0, if condition caused return
+ * 	    -EINTR, if a signal
+ */
+#define poll_event_interruptible(condition, interval)		\
+({								\
+	int __ret = 0;						\
+								\
+	for (;;) {						\
+		if (condition)					\
+			break;					\
+ 		msleep_interruptible(interval);			\
+		if (signal_pending(current)) {			\
+			__ret = -EINTR;				\
+ 			break;					\
+		}						\
+	}							\
+	__ret;
+})
+
+/**
+ * poll_event_interruptible_timeout - check a condition at regular intervals with a timeout, returning early on signals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * @timeout: Time in jiffies to stop checking at
+ * returns 0, if condition caused return
+ * 	   -EINTR, if a signal
+ * 	   -ETIME, if timeout
+ */
+#define poll_event_interruptible_timeout(conditition, interval, timeout)	\
+({										\
+	int __ret = 0;								\
+										\
+	for (;;) {								\
+		if (condition)							\
+			break;							\
+ 		msleep_interruptible(interval);					\
+		if (signal_pending(current)) {					\
+			__ret = -EINTR;						\
+			break;							\
+		}								\
+		if (time_after(jiffies, timeout)) {				\
+			__ret = -ETIME;						\
+			break;							\
+		}								\
+	}									\
+	__ret;									\
+})
+
 #endif /* defined(_LINUX_DELAY_H) */


-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Pavel Machek]

Hi!

> > >         unsigned long timeout = jiffies + HZ/2;
> > >        for (;;) {
> > >                if (ready())
> > >                        return 0;
> > > [IMAGINE HALF A SECOND DELAY HERE]
> > >                if (time_after(timeout, jiffies))
> > >                        break;
> > >                msleep(10);
> > >        }
> > >
> > > Oops
> > 
> > 
> > 
> > There were three proposed solutions though.
> 
> <snip>
> 
> FWIW, here is my poll_event() patch from some time ago updated to
> 2.6.18-rc4. I think it's closest in spirit to 3). Comments, thoughts?
> 
> Description: Add the poll_event(), poll_event_timeout(),
> poll_event_interruptible() and poll_event_interruptible_timeout()
> interfaces. These macros encapsulate typical sleeping code usage in a
> sane way.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> diff -urpN 2.6.18-rc4/include/linux/delay.h 2.6.18-rc4-dev/include/linux/delay.h
> --- 2.6.18-rc4/include/linux/delay.h	2006-08-11 22:40:57.000000000 -0700
> +++ 2.6.18-rc4-dev/include/linux/delay.h	2006-08-11 23:26:23.000000000 -0700
> @@ -44,4 +44,99 @@ static inline void ssleep(unsigned int s
>  	msleep(seconds * 1000);
>  }
>  
> +/*
> + * poll_event* check if a @condition is true every @interval milliseconds,
> + * up to a @timeout maximum, if specified, else forever.
> + * The *_interruptible versions will sleep in TASK_INTERRUPTIBLE
> + * (instead of TASK_UNINTERRUPTIBLE) and will return early on signals
> + * These interfaces are intentionally very similar to wait_event*
> + */
> +
> +/**
> + * poll_event - check a condition at regular intervals
> + * @condition: Condition to check
> + * @interval: Number of milliseconds between checks
> + */
> +#define poll_event(condition, interval)				\
> +do {								\
> +	while (!(condition))					\
> +		msleep(interval);				\
> +} while(0)

Well, someone will want to do usleep here, right? poll_msec()?

> +/**
> + * poll_event_timeout - check a condition at regular intervals with a timeout
> + * @condition: Condition to check
> + * @interval: Number of milliseconds between checks
> + * @timeout: Time in jiffies to stop checking at
> + * returns: 0, if condition caused return
> + * 	    -ETIME, if timeout
> + */
> +#define poll_event_timeout(condition, interval, timeout)		\
> +({									\
> +	int __ret = 0;							\
> + 									\
> +	for (;;) {							\
> +		if (condition)						\
> +			break;						\
> + 		msleep(interval);					\
> +		if (time_after(jiffies, timeout)) {			\
> +			__ret = -ETIME;					\
> + 			break;						\
> +		}							\
> +	}								\
> +	__ret;								\
> +})

This is wrong. See my example above.
> +/**
> + * poll_event_interruptible_timeout - check a condition at regular intervals with a timeout, returning early on signals
> + * @condition: Condition to check
> + * @interval: Number of milliseconds between checks
> + * @timeout: Time in jiffies to stop checking at
> + * returns 0, if condition caused return
> + * 	   -EINTR, if a signal
> + * 	   -ETIME, if timeout
> + */
> +#define poll_event_interruptible_timeout(conditition, interval, timeout)	\
> +({										\
> +	int __ret = 0;								\
> +										\
> +	for (;;) {								\
> +		if (condition)							\
> +			break;							\
> + 		msleep_interruptible(interval);					\
> +		if (signal_pending(current)) {					\
> +			__ret = -EINTR;						\
> +			break;							\
> +		}								\
> +		if (time_after(jiffies, timeout)) {				\
> +			__ret = -ETIME;						\
> +			break;							\
> +		}								\
> +	}									\
> +	__ret;									\
> +})

Wrong.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Darren Jenkins]

Sorry I missed Nishanth Aravamudan's mail

On 8/14/06, Pavel Machek <pavel@suse.cz> wrote:

> > FWIW, here is my poll_event() patch from some time ago updated to
> > 2.6.18-rc4. I think it's closest in spirit to 3). Comments, thoughts?

Was that the right patch ? because it doesn't seem to implement the fix in 3).
I think the easiest fix for this patch would be using number 2)



> > +/**
> > + * poll_event_timeout - check a condition at regular intervals with a timeout
> > + * @condition: Condition to check
> > + * @interval: Number of milliseconds between checks
> > + * @timeout: Time in jiffies to stop checking at
> > + * returns: 0, if condition caused return
> > + *       -ETIME, if timeout
> > + */
> > +#define poll_event_timeout(condition, interval, timeout)             \
> > +({                                                                   \
> > +     int __ret = 0;                                                  \
> > +                                                                     \
> > +     for (;;) {                                                      \
> > +             if (condition)                                          \
> > +                     break;                                          \
> > +             msleep(interval);                                       \
> > +             if (time_after(jiffies, timeout)) {                     \
                             if (condition)
                                     break;
> > +                     __ret = -ETIME;                                 \
> > +                     break;                                          \
> > +             }                                                       \
> > +     }                                                               \
> > +     __ret;                                                          \
> > +})


+#define poll_event_timeout(condition, interval, timeout)             \
+({                                                                   \
+     int __ret = 0;                                                  \
+                                                                     \
+     for (;;) {                                                      \
+             if (condition)                                          \
+                     break;                                          \
+             msleep(interval);                                       \
+             if (time_after(jiffies, timeout)) {                     \
                      if (condition)
                              break;
+                     __ret = -ETIME;                                 \
+                     break;                                          \
+             }                                                       \
+     }                                                               \
+     __ret;                                                          \
+})



> > +/**
> > + * poll_event_interruptible_timeout - check a condition at regular intervals with a timeout, returning early on signals
> > + * @condition: Condition to check
> > + * @interval: Number of milliseconds between checks
> > + * @timeout: Time in jiffies to stop checking at
> > + * returns 0, if condition caused return
> > + *      -EINTR, if a signal
> > + *      -ETIME, if timeout
> > + */
> > +#define poll_event_interruptible_timeout(conditition, interval, timeout)     \
> > +({                                                                           \
> > +     int __ret = 0;                                                          \
> > +                                                                             \
> > +     for (;;) {                                                              \
> > +             if (condition)                                                  \
> > +                     break;                                                  \
> > +             msleep_interruptible(interval);                                 \
> > +             if (signal_pending(current)) {                                  \
> > +                     __ret = -EINTR;                                         \
> > +                     break;                                                  \
> > +             }                                                               \
> > +             if (time_after(jiffies, timeout)) {                             \
                             if (condition)
                                     break;
> > +                     __ret = -ETIME;                                         \
> > +                     break;                                                  \
> > +             }                                                               \
> > +     }                                                                       \
> > +     __ret;                                                                  \
> > +})

Darren J.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Nishanth Aravamudan]

On 14.08.2006 [00:42:12 +0200], Pavel Machek wrote:
> Well, someone will want to do usleep here, right? poll_msec()?

A good point... there is no usleep() that I can see, though. msleep()
uses schedule_timeout() which is at best jiffy-granularity, meaning
order of milliseconds for the foreseeable future. I was going to add a
units suffix to all the macros, but I decided to hold off still for two
reasons: 1) poll_event_interruptible_timeout() is already pretty long :)
and 2) the msecs suffix would only apply to interval parameter, not the
timeout one, which is an absolute time in jiffies. I suppose I could
rework the macros, since they would mostly be used in drivers, to pass
in a relative millisecond value as the timeout and covert it in the
macros to an absolute time in jiffies. Would that be preferred?


On 14.08.2006 [09:41:41 +1000], Darren Jenkins wrote:
> Sorry I missed Nishanth Aravamudan's mail
> 
> On 8/14/06, Pavel Machek <pavel@suse.cz> wrote:
> 
> >> FWIW, here is my poll_event() patch from some time ago updated to
> >> 2.6.18-rc4. I think it's closest in spirit to 3). Comments, thoughts?
> 
> Was that the right patch ? because it doesn't seem to implement the fix in 
> 3).
> I think the easiest fix for this patch would be using number 2)

Yes, that was my fault, I sent the wrong patch. That's what I get for
sending a patch on a Friday night :)

Updated patch, which is what you changed, Darren, with a small
difference (checking for !condition, rather than condition) is below.

Thanks,
Nish

Description: Add the poll_event(), poll_event_timeout(),
poll_event_interruptible() and poll_event_interruptible_timeout()
interfaces. These macros encapsulate typical sleeping code usage in a
sane way, accounting for a common bug which could indicate a timeout
when, in fact, the condition is true (and takes precedent).

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

diff -urpN 2.6.18-rc4/include/linux/delay.h 2.6.18-rc4-dev/include/linux/delay.h
--- 2.6.18-rc4/include/linux/delay.h	2006-08-11 22:40:57.000000000 -0700
+++ 2.6.18-rc4-dev/include/linux/delay.h	2006-08-13 21:06:28.000000000 -0700
@@ -44,4 +44,101 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+/*
+ * poll_event* check if a @condition is true every @interval milliseconds,
+ * up to a @timeout maximum, if specified, else forever.
+ * The *_interruptible versions will sleep in TASK_INTERRUPTIBLE
+ * (instead of TASK_UNINTERRUPTIBLE) and will return early on signals
+ * These interfaces are intentionally very similar to wait_event*
+ */
+
+/**
+ * poll_event - check a condition at regular intervals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ */
+#define poll_event(condition, interval)				\
+do {								\
+	while (!(condition))					\
+		msleep(interval);				\
+} while(0)
+
+/**
+ * poll_event_timeout - check a condition at regular intervals with a timeout
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * @timeout: Time in jiffies to stop checking at
+ * returns: 0, if condition caused return
+ * 	    -ETIME, if timeout
+ */
+#define poll_event_timeout(condition, interval, timeout)		\
+({									\
+	int __ret = 0;							\
+ 									\
+	for (;;) {							\
+		if (condition)						\
+			break;						\
+ 		msleep(interval);					\
+		if (time_after(jiffies, timeout)) {			\
+ 			if (!(condition))				\
+				__ret = -ETIME;				\
+ 			break;						\
+		}							\
+	}								\
+	__ret;								\
+})
+
+/**
+ * poll_event_interruptible - check a condition at regular intervals, returning early on signals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * returns: 0, if condition caused return
+ * 	    -EINTR, if a signal
+ */
+#define poll_event_interruptible(condition, interval)		\
+({								\
+	int __ret = 0;						\
+								\
+	for (;;) {						\
+		if (condition)					\
+			break;					\
+ 		msleep_interruptible(interval);			\
+		if (signal_pending(current)) {			\
+			__ret = -EINTR;				\
+ 			break;					\
+		}						\
+	}							\
+	__ret;
+})
+
+/**
+ * poll_event_interruptible_timeout - check a condition at regular intervals with a timeout, returning early on signals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * @timeout: Time in jiffies to stop checking at
+ * returns 0, if condition caused return
+ * 	   -EINTR, if a signal
+ * 	   -ETIME, if timeout
+ */
+#define poll_event_interruptible_timeout(conditition, interval, timeout)	\
+({										\
+	int __ret = 0;								\
+										\
+	for (;;) {								\
+		if (condition)							\
+			break;							\
+ 		msleep_interruptible(interval);					\
+		if (signal_pending(current)) {					\
+			__ret = -EINTR;						\
+			break;							\
+		}								\
+		if (time_after(jiffies, timeout)) {				\
+ 			if (!(condition))					\
+				__ret = -ETIME;					\
+			break;							\
+		}								\
+	}									\
+	__ret;									\
+})
+
 #endif /* defined(_LINUX_DELAY_H) */

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Darren Jenkins]

G'day,

On 8/14/06, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> On 14.08.2006 [00:42:12 +0200], Pavel Machek wrote:
> > Well, someone will want to do usleep here, right? poll_msec()?
>
> A good point... there is no usleep() that I can see, though. msleep()
> uses schedule_timeout() which is at best jiffy-granularity, meaning
> order of milliseconds for the foreseeable future. I was going to add a
> units suffix to all the macros, but I decided to hold off still for two
> reasons: 1) poll_event_interruptible_timeout() is already pretty long :)
> and 2) the msecs suffix would only apply to interval parameter, not the
> timeout one, which is an absolute time in jiffies. I suppose I could
> rework the macros, since they would mostly be used in drivers, to pass
> in a relative millisecond value as the timeout and covert it in the
> macros to an absolute time in jiffies. Would that be preferred?

I think while you are hiding complexity in a macro, you should hide
the absolute time to jiffies conversion complexity(although not that
complex) too. It _is_ closely related to the interfaces you are
writing and in a lot of cases will probably be the only place the
conversion is used. So I say, bung it in there.


> Updated patch, which is what you changed, Darren, with a small
> difference (checking for !condition, rather than condition) is below.
>
> Thanks,
> Nish

This patch looks much better. Can't see anything wrong with it.

Also after seeing the problems people have had with polling loops
these macro's do seem like a good idea.


Darren J.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Nishanth Aravamudan]

On 14.08.2006 [14:34:58 +1000], Darren Jenkins wrote:
> G'day,
> 
> On 8/14/06, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> >On 14.08.2006 [00:42:12 +0200], Pavel Machek wrote:
> >> Well, someone will want to do usleep here, right? poll_msec()?
> >
> >A good point... there is no usleep() that I can see, though. msleep()
> >uses schedule_timeout() which is at best jiffy-granularity, meaning
> >order of milliseconds for the foreseeable future. I was going to add a
> >units suffix to all the macros, but I decided to hold off still for two
> >reasons: 1) poll_event_interruptible_timeout() is already pretty long :)
> >and 2) the msecs suffix would only apply to interval parameter, not the
> >timeout one, which is an absolute time in jiffies. I suppose I could
> >rework the macros, since they would mostly be used in drivers, to pass
> >in a relative millisecond value as the timeout and covert it in the
> >macros to an absolute time in jiffies. Would that be preferred?
> 
> I think while you are hiding complexity in a macro, you should hide
> the absolute time to jiffies conversion complexity(although not that
> complex) too. It _is_ closely related to the interfaces you are
> writing and in a lot of cases will probably be the only place the
> conversion is used. So I say, bung it in there.

Ok, below is my attempt at incorporating this. Also fixed a typo in
poll_event_interruptible_timeout() and made a similar fix as 2) for
the interruptible variants, where the condition takes precendence over a
signal.

> >Updated patch, which is what you changed, Darren, with a small
> >difference (checking for !condition, rather than condition) is below.
> >
> >Thanks,
> >Nish
> 
> This patch looks much better. Can't see anything wrong with it.
> 
> Also after seeing the problems people have had with polling loops
> these macro's do seem like a good idea.

That was the thought many kernels ago, as well :)

Description: Add the poll_event(), poll_event_timeout(),
poll_event_interruptible() and poll_event_interruptible_timeout()
interfaces. These macros encapsulate typical sleeping code usage in a
sane way, accounting for a common bug which could indicate a timeout
when, in fact, the condition is true (and always takes precedent when
returning to the caller).

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

diff -urpN 2.6.18-rc4/include/linux/delay.h 2.6.18-rc4-dev/include/linux/delay.h
--- 2.6.18-rc4/include/linux/delay.h	2006-08-11 22:40:57.000000000 -0700
+++ 2.6.18-rc4-dev/include/linux/delay.h	2006-08-13 22:26:14.000000000 -0700
@@ -44,4 +44,107 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+/*
+ * poll_event* check if a @condition is true every @interval milliseconds,
+ * up to a @timeout maximum, if specified, else forever.
+ * The *_interruptible versions will sleep in TASK_INTERRUPTIBLE
+ * (instead of TASK_UNINTERRUPTIBLE) and will return early on signals
+ * These interfaces are intentionally very similar to wait_event*
+ */
+
+/**
+ * poll_event - check a condition at regular intervals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ */
+#define poll_event(condition, interval)				\
+do {								\
+	while (!(condition))					\
+		msleep(interval);				\
+} while(0)
+
+/**
+ * poll_event_timeout - check a condition at regular intervals with a timeout
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * @timeout: Number of milliseconds from now to stop checking at
+ * returns: 0, if condition caused return
+ * 	    -ETIME, if timeout
+ */
+#define poll_event_timeout(condition, interval, timeout)		\
+({									\
+	int __ret = 0;							\
+ 	unsigned long __timeout = jiffies +				\
+ 					msecs_to_jiffies(timeout) + 1;	\
+ 									\
+	for (;;) {							\
+		if (condition)						\
+			break;						\
+ 		msleep(interval);					\
+		if (time_after(jiffies, __timeout)) {			\
+ 			if (!(condition))				\
+				__ret = -ETIME;				\
+ 			break;						\
+		}							\
+	}								\
+	__ret;								\
+})
+
+/**
+ * poll_event_interruptible - check a condition at regular intervals, returning early on signals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * returns: 0, if condition caused return
+ * 	    -EINTR, if a signal
+ */
+#define poll_event_interruptible(condition, interval)		\
+({								\
+	int __ret = 0;						\
+								\
+	for (;;) {						\
+		if (condition)					\
+			break;					\
+ 		msleep_interruptible(interval);			\
+		if (signal_pending(current)) {			\
+ 			if (!(condition))			\
+				__ret = -EINTR;			\
+ 			break;					\
+		}						\
+	}							\
+	__ret;
+})
+
+/**
+ * poll_event_interruptible_timeout - check a condition at regular intervals with a timeout, returning early on signals
+ * @condition: Condition to check
+ * @interval: Number of milliseconds between checks
+ * @timeout: Number of milliseconds from now to stop checking at
+ * returns 0, if condition caused return
+ * 	   -EINTR, if a signal
+ * 	   -ETIME, if timeout
+ */
+#define poll_event_interruptible_timeout(condition, interval, timeout)	\
+({									\
+	int __ret = 0;							\
+ 	unsigned long __timeout = jiffies +				\
+ 					msecs_to_jiffies(timeout) + 1;	\
+									\
+	for (;;) {							\
+		if (condition)						\
+			break;						\
+ 		msleep_interruptible(interval);				\
+		if (signal_pending(current)) {				\
+ 			if (!(condition))				\
+				__ret = -EINTR;				\
+			break;						\
+		}							\
+		if (time_after(jiffies, __timeout)) {			\
+ 			if (!(condition))				\
+				__ret = -ETIME;				\
+			break;						\
+		}							\
+	}								\
+	__ret;								\
+})
+
 #endif /* defined(_LINUX_DELAY_H) */

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Pavel Machek]

Hi!

> > I think while you are hiding complexity in a macro, you should hide
> > the absolute time to jiffies conversion complexity(although not that
> > complex) too. It _is_ closely related to the interfaces you are
> > writing and in a lot of cases will probably be the only place the
> > conversion is used. So I say, bung it in there.
> 
> Ok, below is my attempt at incorporating this. Also fixed a typo in
> poll_event_interruptible_timeout() and made a similar fix as 2) for
> the interruptible variants, where the condition takes precendence over a
> signal.

Agreed, looks okay to me.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Nishanth Aravamudan]

On 15.08.2006 [12:08:09 +0200], Pavel Machek wrote:
> Hi!
> 
> > > I think while you are hiding complexity in a macro, you should hide
> > > the absolute time to jiffies conversion complexity(although not that
> > > complex) too. It _is_ closely related to the interfaces you are
> > > writing and in a lot of cases will probably be the only place the
> > > conversion is used. So I say, bung it in there.
> > 
> > Ok, below is my attempt at incorporating this. Also fixed a typo in
> > poll_event_interruptible_timeout() and made a similar fix as 2) for
> > the interruptible variants, where the condition takes precendence over a
> > signal.
> 
> Agreed, looks okay to me.

Thanks, Pavel.

Andrej, care to rework your patch using these macros, if possible?

Thanks,
Nish

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix

[Andrej Zaikin]

2006/8/16, Nishanth Aravamudan <nacc@us.ibm.com>:
> On 15.08.2006 [12:08:09 +0200], Pavel Machek wrote:
> > Hi!
> >
> > > > I think while you are hiding complexity in a macro, you should hide
> > > > the absolute time to jiffies conversion complexity(although not that
> > > > complex) too. It _is_ closely related to the interfaces you are
> > > > writing and in a lot of cases will probably be the only place the
> > > > conversion is used. So I say, bung it in there.
> > >
> > > Ok, below is my attempt at incorporating this. Also fixed a typo in
> > > poll_event_interruptible_timeout() and made a similar fix as 2) for
> > > the interruptible variants, where the condition takes precendence over a
> > > signal.
> >
> > Agreed, looks okay to me.
>
> Thanks, Pavel.
>
> Andrej, care to rework your patch using these macros, if possible?

Okay, I'll try
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors