Add poll_event* interfaces (was Re: [patch] fix common
PATCH 1/1 Add poll_event* interfaces (was Re: [patch] fix common
Quoted 38 lines · Darren Jenkins
G'day
Quoted 15 lines · Alexey Dobriyan
Quoted 14 lines · Darren Jenkins
Quoted 11 lines · Pavel Machek
Quoted 8 lines
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.
PavelQuoted 7 lines
Quoted 4 lines
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.
Quoted 10 lines
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); }
Quoted 5 lines
Quoted 1 line
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.
include/linux/delay.h +95 -0
@@ -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) */
Signature · 6 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center _______________________________________________ Kernel-janitors mailing list [email protected] https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
Hi!
Quoted 43 lines
Quoted 10 lines
Quoted 10 lines
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); }Quoted 5 lines
Quoted 1 line
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()andpoll_event_interruptible_timeout()interfaces. These macros encapsulate typical sleeping code usage in a sane way.Signed-off-by Nishanth Aravamudan <[email protected]>diff -urpN
2.6.18-rc4/include/linux/delay.h2.6.18-rc4-dev/include/linux/delay.hinclude/linux/delay.h +8
-0@@ -44,4 +44,99 @@ static inline void ssleep(unsigned int smsleep (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* +*/ +
include/linux/delay.h static inline void ssleep(unsigned int s · line 64
Quoted 10 lines
include/linux/delay.h +10
-0+/** +* 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()?
Quoted 13 lines
include/linux/delay.h +13
-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 (;;) { \
include/linux/delay.h static inline void ssleep(unsigned int s · line 88
Quoted 10 lines
include/linux/delay.h +10
-0+if (condition) \ +break ; \ +msleep (interval); \ +if (time_after (jiffies, timeout)) { \ + __ret = -ETIME ; \ +break ; \ + } \ + } \ + __ret; \ +})
This is wrong. See my example above.
Quoted 18 lines
include/linux/delay.h +18
-0+/** +* 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)) { \
include/linux/delay.h static inline void ssleep(unsigned int s · line 140
Quoted 10 lines
include/linux/delay.h +10
-0+ __ret = -EINTR ; \ +break ; \ + } \ +if (time_after (jiffies, timeout)) { \ + __ret = -ETIME ; \ +break ; \ + } \ + } \ + __ret; \ +})
Wrong.
Pavel
Signature · 6 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html _______________________________________________ Kernel-janitors mailing list [email protected] https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
In reply to Nishanth AravamudanSorry I missed Nishanth Aravamudan's mail
Quoted 2 lines · Pavel Machek
Quoted 2 lines
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)
Quoted 9 lines
Quoted 9 lines
include/linux/delay.h +9
-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) \
Quoted 8 lines
include/linux/delay.h static inline void ssleep(unsigned int s · line 82
Quoted 8 lines
include/linux/delay.h +8
-0+({ \ + int __ret = 0; \ + \ + for (;;) { \ + if (condition) \ + break; \ + msleep(interval); \ + if (time_after(jiffies, timeout)) { \
if (condition)
break;Quoted 6 lines
include/linux/delay.h static inline void ssleep(unsigned int s · line 88
Quoted 6 lines
include/linux/delay.h +6
-0+ __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; \+})
Quoted 14 lines
Quoted 14 lines
include/linux/delay.h +14
-0+/** +* 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 (;;) { \
Quoted 8 lines
include/linux/delay.h static inline void ssleep(unsigned int s · line 134
Quoted 8 lines
include/linux/delay.h +8
-0+if (condition) \ +break ; \ +msleep_interruptible (interval); \ +if (signal_pending (current)) { \ + __ret = -EINTR ; \ +break ; \ + } \ +if (time_after (jiffies, timeout)) { \
if (condition)
break;Quoted 6 lines
include/linux/delay.h static inline void ssleep(unsigned int s · line 88
Quoted 6 lines
include/linux/delay.h +6
-0+ __ret = -ETIME ; \ +break ; \ + } \ + } \ + __ret; \ +})
Darren J.
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
In reply to Nishanth AravamudanQuoted 1 line · Pavel Machek
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?
Quoted 10 lines · Darren Jenkins
Sorry I missed Nishanth Aravamudan's mail
Quoted 2 lines · Pavel Machek
Quoted 2 lines
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).
include/linux/delay.h +97 -0
@@ -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) */
Signature · 6 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center _______________________________________________ Kernel-janitors mailing list [email protected] https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
In reply to Nishanth AravamudanG'day,
Quoted 13 lines · Nishanth Aravamudan
Quoted 1 line · Pavel Machek
Well, someone will want to do usleep here, right?
poll_msec()?A good point... there is no
usleep()that I can see, though.msleep()usesschedule_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.
Quoted 5 lines
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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
In reply to Nishanth AravamudanQuoted 17 lines · Darren Jenkins
G'day,
Quoted 13 lines · Nishanth Aravamudan
Quoted 1 line · Pavel Machek
Well, someone will want to do usleep here, right?
poll_msec()?A good point... there is no
usleep()that I can see, though.msleep()usesschedule_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?
Quoted 5 lines
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.
Quoted 10 lines
Quoted 5 lines
Updated patch, which is what you changed, Darren, with a small difference (checking for !condition, rather than condition) is below.
Thanks,
NishThis 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).
include/linux/delay.h +103 -0
@@ -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) */
Signature · 6 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center _______________________________________________ Kernel-janitors mailing list [email protected] https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
In reply to Nishanth AravamudanHi!
Quoted 10 lines
Quoted 5 lines
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
Signature · 6 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html _______________________________________________ Kernel-janitors mailing list [email protected] https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
In reply to Nishanth AravamudanQuoted 8 lines · Pavel Machek
Hi!
Quoted 6 lines
Quoted 5 lines
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.
Quoted 6 lines
Quoted 4 lines
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
Signature · 6 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center _______________________________________________ Kernel-janitors mailing list [email protected] https://lists.osdl.org/mailman/listinfo/kernel-janitors
Re: [KJ] [PATCH] Add poll_event* interfaces (was Re: [patch] fix
In reply to Nishanth Aravamudan2006/8/16, Nishanth Aravamudan <[email protected]>:
Quoted 14 lines · Pavel Machek
Quoted 13 lines
Hi!
Quoted 10 lines
Quoted 5 lines
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.
Quoted 5 lines
Quoted 1 line
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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors