* [PATCH]: DELAYTIMER_MAX is defined
@ 2003-05-06 16:33 Eric Piel
2003-05-06 20:12 ` george anzinger
0 siblings, 1 reply; 9+ messages in thread
From: Eric Piel @ 2003-05-06 16:33 UTC (permalink / raw)
To: george anzinger; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 745 bytes --]
Hello,
Playing around with the posix timers I've noticed that DELAYTIMER_MAX is
not defined. This constant is specified in the POSIX specifications. It
should contain the maximum possible value of overruns on a signal. It is
also said that the overrun shouldn't overflow. cf
http://www.opengroup.org/onlinepubs/007904975/functions/timer_getoverrun.html
So here is a patch to add this constant and a check that the overrun
variable never overflow. It's for 2.5.67 but should apply flawlessly to
2.5.69 too.
Actually one could wonder if the test on the overflow is really needed
has an overrun reaching 2^31 seems very hard to happen. However the
constant definition is not hurting anything and looks necessary for
closer POSIX compliance.
Eric
[-- Attachment #2: delaytimer_max-const.patch --]
[-- Type: text/plain, Size: 1274 bytes --]
diff -ur linux-2.5.67-ia64-hrtcore/include/linux/limits.h linux-2.5.67-ia64-hrtimers/include/linux/limits.h
--- linux-2.5.67-ia64-hrtcore/include/linux/limits.h 2003-04-22 11:10:44.000000000 +0200
+++ linux-2.5.67-ia64-hrtimers/include/linux/limits.h 2003-05-06 13:45:48.000000000 +0200
@@ -17,6 +17,8 @@
#define XATTR_SIZE_MAX 65536 /* size of an extended attribute value (64k) */
#define XATTR_LIST_MAX 65536 /* size of extended attribute namelist (64k) */
+#define DELAYTIMER_MAX INT_MAX /* # timer expiration overruns a POSIX.1b timer may have */
+
#define RTSIG_MAX 32
#endif
diff -ur linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h
--- linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h 2003-04-22 11:10:44.000000000 +0200
+++ linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h 2003-05-06 16:07:56.000000000 +0200
@@ -25,6 +59,7 @@
#define posix_bump_timer(timr) do { \
(timr)->it_timer.expires += (timr)->it_incr; \
- (timr)->it_overrun++; \
+ if ((timr)->it_overrun < DELAYTIMER_MAX)\
+ (timr)->it_overrun++; \
}while (0)
#endif
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-06 16:33 [PATCH]: DELAYTIMER_MAX is defined Eric Piel @ 2003-05-06 20:12 ` george anzinger 2003-05-07 7:29 ` Eric Piel 0 siblings, 1 reply; 9+ messages in thread From: george anzinger @ 2003-05-06 20:12 UTC (permalink / raw) To: Eric Piel; +Cc: linux-kernel Eric Piel wrote: > Hello, > > Playing around with the posix timers I've noticed that DELAYTIMER_MAX is > not defined. This constant is specified in the POSIX specifications. It > should contain the maximum possible value of overruns on a signal. It is > also said that the overrun shouldn't overflow. cf > http://www.opengroup.org/onlinepubs/007904975/functions/timer_getoverrun.html > > So here is a patch to add this constant and a check that the overrun > variable never overflow. It's for 2.5.67 but should apply flawlessly to > 2.5.69 too. > Actually one could wonder if the test on the overflow is really needed > has an overrun reaching 2^31 seems very hard to happen. However the > constant definition is not hurting anything and looks necessary for > closer POSIX compliance. I think this is needed in glibc. I am not sure how that relates to kernel defines. -g > > Eric > > > ------------------------------------------------------------------------ > > diff -ur linux-2.5.67-ia64-hrtcore/include/linux/limits.h linux-2.5.67-ia64-hrtimers/include/linux/limits.h > --- linux-2.5.67-ia64-hrtcore/include/linux/limits.h 2003-04-22 11:10:44.000000000 +0200 > +++ linux-2.5.67-ia64-hrtimers/include/linux/limits.h 2003-05-06 13:45:48.000000000 +0200 > @@ -17,6 +17,8 @@ > #define XATTR_SIZE_MAX 65536 /* size of an extended attribute value (64k) */ > #define XATTR_LIST_MAX 65536 /* size of extended attribute namelist (64k) */ > > +#define DELAYTIMER_MAX INT_MAX /* # timer expiration overruns a POSIX.1b timer may have */ > + > #define RTSIG_MAX 32 > > #endif > diff -ur linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h > --- linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h 2003-04-22 11:10:44.000000000 +0200 > +++ linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h 2003-05-06 16:07:56.000000000 +0200 > @@ -25,6 +59,7 @@ > > #define posix_bump_timer(timr) do { \ > (timr)->it_timer.expires += (timr)->it_incr; \ > - (timr)->it_overrun++; \ > + if ((timr)->it_overrun < DELAYTIMER_MAX)\ > + (timr)->it_overrun++; \ > }while (0) > #endif > -- George Anzinger george@mvista.com High-res-timers: http://sourceforge.net/projects/high-res-timers/ Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-06 20:12 ` george anzinger @ 2003-05-07 7:29 ` Eric Piel 2003-05-07 7:48 ` Ulrich Drepper 0 siblings, 1 reply; 9+ messages in thread From: Eric Piel @ 2003-05-07 7:29 UTC (permalink / raw) To: george anzinger; +Cc: linux-kernel george anzinger wrote: > > Eric Piel wrote: > > Playing around with the posix timers I've noticed that DELAYTIMER_MAX is > > not defined. This constant is specified in the POSIX specifications. It > > should contain the maximum possible value of overruns on a signal. It is > > also said that the overrun shouldn't overflow. cf > > http://www.opengroup.org/onlinepubs/007904975/functions/timer_getoverrun.html > > > > So here is a patch to add this constant and a check that the overrun > > variable never overflow. It's for 2.5.67 but should apply flawlessly to > > 2.5.69 too. > I think this is needed in glibc. I am not sure how that relates to > kernel defines. How can the glibc do things like that? : > > #define posix_bump_timer(timr) do { \ > > (timr)->it_timer.expires += (timr)->it_incr; \ > > - (timr)->it_overrun++; \ > > + if ((timr)->it_overrun < DELAYTIMER_MAX)\ > > + (timr)->it_overrun++; \ > > }while (0) In addition knowing that DELAYTIMER_MAX is dependant on the implementation and that the implementation is now in the kernel I think DELAYTIMER_MAX should be defined by the kernel. Then glibc can do its usual trick of stealing all the constant from the kernel... Eric ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-07 7:29 ` Eric Piel @ 2003-05-07 7:48 ` Ulrich Drepper 2003-05-07 12:00 ` Eric Piel 0 siblings, 1 reply; 9+ messages in thread From: Ulrich Drepper @ 2003-05-07 7:48 UTC (permalink / raw) To: Eric Piel; +Cc: george anzinger, linux-kernel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Eric Piel wrote: >>>Playing around with the posix timers I've noticed that DELAYTIMER_MAX is >>>not defined. This constant is specified in the POSIX specifications. It >>>should contain the maximum possible value of overruns on a signal. It is >>>also said that the overrun shouldn't overflow. cf >>>http://www.opengroup.org/onlinepubs/007904975/functions/timer_getoverrun.html This is not correct. The constant does not have to be defined. Like all the various *_MAX constants they only have to be defined if there is a fixed limit the implementation has. If there is none or it can only be defined dynamically at runtime the the macro must not be defined. Instead sysconf() can provide the value. But not even this is necessary. sysconf() can return -1. Anyway, in this specific case the implementation should be protected against the ever so improbable overflow of the counter, yes. If you want a fixed value, fine. If you want to use ULONG_MAX (or whatever), good too. Whether we advertise this limit is another thing. Advertising it in the macro means it never can be changed. - -- - --------------. ,-. 444 Castro Street Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA Red Hat `--' drepper at redhat.com `--------------------------- -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQE+uLpr2ijCOnn/RHQRAgiAAKCIj4hn7m/lkOIrLjHjqirTFPfvhQCeLbxB cl9pJ+BTHy7VQzpCDeyjmSs= =WTUD -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-07 7:48 ` Ulrich Drepper @ 2003-05-07 12:00 ` Eric Piel 2003-05-07 15:02 ` george anzinger 0 siblings, 1 reply; 9+ messages in thread From: Eric Piel @ 2003-05-07 12:00 UTC (permalink / raw) To: Ulrich Drepper; +Cc: george anzinger, linux-kernel Ulrich Drepper wrote: > This is not correct. The constant does not have to be defined. Like > all the various *_MAX constants they only have to be defined if there is > a fixed limit the implementation has. If there is none or it can only > be defined dynamically at runtime the the macro must not be defined. > Instead sysconf() can provide the value. But not even this is > necessary. sysconf() can return -1. Sorry, I wasn't aware about this POSIX rule, thank you for pointing out this. Knowing that it would obviously be better providing support of the constant _SC_DELAYTIMER_MAX for sysconf() and return a value. This would imply there is now some job for glibc. However it's still unclear for me how the glibc would know about wich value it should return while this question would be so easy to answer for the kernel! Also AFAIK, right now, only the NPTL supports the new syscalls for the timers (and none supports the clocks syscalls). Does it mean a special __sysconf() is necessary in the NPTL? Well, probably that's why you suggested -1 as a return value I guess :-) > > Anyway, in this specific case the implementation should be protected > against the ever so improbable overflow of the counter, yes. If you > want a fixed value, fine. If you want to use ULONG_MAX (or whatever), > good too. Whether we advertise this limit is another thing. > Advertising it in the macro means it never can be changed. > You are right also here, of course with this point of view it's better not putting any constant. So the patch could become something like that: ? diff -ur linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h --- linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h 2003-04-22 11:10:44.000000000 +0200 +++ linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h 2003-05-06 16:07:56.000000000 +0200 @@ -25,6 +59,7 @@ #define posix_bump_timer(timr) do { \ (timr)->it_timer.expires += (timr)->it_incr; \ - (timr)->it_overrun++; \ + if ((timr)->it_overrun < INT_MAX)\ + (timr)->it_overrun++; \ }while (0) #endif Eric ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-07 12:00 ` Eric Piel @ 2003-05-07 15:02 ` george anzinger 2003-05-07 18:21 ` Ulrich Drepper 0 siblings, 1 reply; 9+ messages in thread From: george anzinger @ 2003-05-07 15:02 UTC (permalink / raw) To: Eric Piel; +Cc: Ulrich Drepper, linux-kernel I would like to give some thought to setting this value rather low, say 1000 or so. The issue is that the update to the expire time is done by adding the timer increment repeatedly until a time that has not yet passed is found. THIS TAKES TIME and it is done under an irq spin_lock! I do think we should limit the time in this loop and, also, possibly finding a way to do it with out holding the spin_lock, so that other tasks could at least preempt the caller. (As it is, it already done in the callers task and not in an interrupt context.) This, of course, also says that we should not only limit the value of overrun, but also do something different when it happens. -g Eric Piel wrote: > Ulrich Drepper wrote: > >>This is not correct. The constant does not have to be defined. Like >>all the various *_MAX constants they only have to be defined if there is >>a fixed limit the implementation has. If there is none or it can only >>be defined dynamically at runtime the the macro must not be defined. >>Instead sysconf() can provide the value. But not even this is >>necessary. sysconf() can return -1. > > Sorry, I wasn't aware about this POSIX rule, thank you for pointing out > this. Knowing that it would obviously be better providing support of the > constant _SC_DELAYTIMER_MAX for sysconf() and return a value. > > This would imply there is now some job for glibc. However it's still > unclear for me how the glibc would know about wich value it should > return while this question would be so easy to answer for the kernel! > Also AFAIK, right now, only the NPTL supports the new syscalls for the > timers (and none supports the clocks syscalls). Does it mean a special > __sysconf() is necessary in the NPTL? Well, probably that's why you > suggested -1 as a return value I guess :-) > > >>Anyway, in this specific case the implementation should be protected >>against the ever so improbable overflow of the counter, yes. If you >>want a fixed value, fine. If you want to use ULONG_MAX (or whatever), >>good too. Whether we advertise this limit is another thing. >>Advertising it in the macro means it never can be changed. >> > > You are right also here, of course with this point of view it's > better not putting any constant. > So the patch could become something like that: ? > > diff -ur linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h > --- linux-2.5.67-ia64-hrtcore/include/linux/posix-timers.h 2003-04-22 11:10:44.000000000 +0200 > +++ linux-2.5.67-ia64-hrtimers/include/linux/posix-timers.h 2003-05-06 16:07:56.000000000 +0200 > @@ -25,6 +59,7 @@ > > #define posix_bump_timer(timr) do { \ > (timr)->it_timer.expires += (timr)->it_incr; \ > - (timr)->it_overrun++; \ > + if ((timr)->it_overrun < INT_MAX)\ > + (timr)->it_overrun++; \ > }while (0) > #endif > > Eric > > -- George Anzinger george@mvista.com High-res-timers: http://sourceforge.net/projects/high-res-timers/ Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-07 15:02 ` george anzinger @ 2003-05-07 18:21 ` Ulrich Drepper 2003-05-07 18:47 ` george anzinger 0 siblings, 1 reply; 9+ messages in thread From: Ulrich Drepper @ 2003-05-07 18:21 UTC (permalink / raw) To: george anzinger; +Cc: Eric Piel, linux-kernel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 george anzinger wrote: > This, of course, also says that we should not only limit the value of > overrun, but also do something different when it happens. That's legitimate and in this case we perhaps should publish this lower value since it can be limiting. Since real work is involved in having higher values this constiutes another possible DoS in the code and has to be resolved. - -- - --------------. ,-. 444 Castro Street Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA Red Hat `--' drepper at redhat.com `--------------------------- -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQE+uU6P2ijCOnn/RHQRAralAJ43h7EBj8sqm2QmmdTGXwBiAL9LUQCgignS Lu4ITL+4lDmZXoK0BR8UFLU= =DKUl -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-07 18:21 ` Ulrich Drepper @ 2003-05-07 18:47 ` george anzinger 2003-05-07 19:03 ` Ulrich Drepper 0 siblings, 1 reply; 9+ messages in thread From: george anzinger @ 2003-05-07 18:47 UTC (permalink / raw) To: Ulrich Drepper; +Cc: Eric Piel, linux-kernel Ulrich Drepper wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > george anzinger wrote: > > >>This, of course, also says that we should not only limit the value of >>overrun, but also do something different when it happens. > > > That's legitimate and in this case we perhaps should publish this lower > value since it can be limiting. Since real work is involved in having > higher values this constitutes another possible DoS in the code and has > to be resolved. As soon as I get a few other fires under control, I will be working on this and the limit on number of timers issue. Andrew Morton suggested an rlimit for that. Perhaps that is what should be used here also. Seem reasonable? -- George Anzinger george@mvista.com High-res-timers: http://sourceforge.net/projects/high-res-timers/ Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]: DELAYTIMER_MAX is defined 2003-05-07 18:47 ` george anzinger @ 2003-05-07 19:03 ` Ulrich Drepper 0 siblings, 0 replies; 9+ messages in thread From: Ulrich Drepper @ 2003-05-07 19:03 UTC (permalink / raw) To: george anzinger; +Cc: Eric Piel, linux-kernel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 george anzinger wrote: > Andrew Morton suggested > an rlimit for that. Perhaps that is what should be used here also. > > Seem reasonable? For this as well? We are getting an awful lot of these it seems. The DELAYTIMER_MAX thing probably can be hardcoded. I can see no harm in limiting it to 1000 or so. The number of timers can be handled via rlimit. - -- - --------------. ,-. 444 Castro Street Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA Red Hat `--' drepper at redhat.com `--------------------------- -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQE+uViH2ijCOnn/RHQRApl3AKC6l4zZMsn7SaiGSw6hb9KZpc5cQACgm1NW 09sC2WWGZsw1QAUxcTjo8rI= =Nafv -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2003-05-07 18:51 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-05-06 16:33 [PATCH]: DELAYTIMER_MAX is defined Eric Piel 2003-05-06 20:12 ` george anzinger 2003-05-07 7:29 ` Eric Piel 2003-05-07 7:48 ` Ulrich Drepper 2003-05-07 12:00 ` Eric Piel 2003-05-07 15:02 ` george anzinger 2003-05-07 18:21 ` Ulrich Drepper 2003-05-07 18:47 ` george anzinger 2003-05-07 19:03 ` Ulrich Drepper
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).