Currently 3 reserve itimers are allocated purely for kernel use, but unfortunately this breaks the SUSv4 requirement which states: {TIMER_MAX} Maximum number of timers per process supported by the implementation. Minimum Acceptable Value: {_POSIX_TIMER_MAX} .. {_POSIX_TIMER_MAX} The per-process number of timers. Value: 32 Example (from testcases/open_posix_testsuite/conformance/behavior/timers in the LTP package): $ ./1-1.run-test [29] timer_create() did not return success: Resource temporarily unavailable After applying this patch, things function as expected: $ ./1-1.run-test Test PASSED Fix: Patch attached with submission follows: How-To-Repeat: 1. Grab LTP: the package needs to be July or later as I didn't apply many fixes to the branch until post that date -- this may require grabbing the source from git. See the sourceforge page for more info (http://sf.net/projects/ltp). Use ltp/ltp-dev instead of ltp/ltp in the directions, if ltp-dev.git still exists (I'm working on fixing that, because the directions on the webpage are incorrect). 2. Go to the open_posix_testsuite directory: cd $LTP_DIR/testcases/open_posix_testsuite 3. Generate the Makefiles: make generate-makefiles 4. Build the test: cd conformance/behavior/timers; make 5. Execute the test like: ./1-1.run-test
Sorry... some noise from nanosleep was in the last patch (another bug I filed -- kern/149980). This patch only deals with the items I described. Thanks, -Garrett
Responsible Changed From-To: freebsd-bugs->mav If I remember correctly Alexander Motin is working on timers. He might know how to handle this PR.
Hi. Sorry, but this is not my area of knowledge yet. I am working on kernel time event producing, while this is consuming, half way to user-level. Still few comments from quick review: - patch seems slightly incomplete. At least in itimers_alloc() loop condition need to be changed. - To reduce chance of alike mistakes I would implement that patch in opposite way, by defining TIMER_MAX as (32 + TIMER_RESERVED), but reporting to user-level as TIMER_MAX - TIMER_RESERVED. - I have looked on setitimer() implementation referenced here and found no any interaction with timer_create() API and TIMER_MAX. There is indeed many loose ends which make me thing there was or intended to be something, but I haven't found it. Have I missed something? - this code was written by David Xu. I would asked him. -- Alexander Motin
Alexander Motin wrote: > Hi. > > Sorry, but this is not my area of knowledge yet. I am working on kernel > time event producing, while this is consuming, half way to user-level. > > Still few comments from quick review: > - patch seems slightly incomplete. At least in itimers_alloc() loop > condition need to be changed. > - To reduce chance of alike mistakes I would implement that patch in > opposite way, by defining TIMER_MAX as (32 + TIMER_RESERVED), but > reporting to user-level as TIMER_MAX - TIMER_RESERVED. > - I have looked on setitimer() implementation referenced here and found > no any interaction with timer_create() API and TIMER_MAX. There is > indeed many loose ends which make me thing there was or intended to be > something, but I haven't found it. Have I missed something? > - this code was written by David Xu. I would asked him. > Yes, the code was written by me long time ago, my original intention was to not only implement timer_create() but also merge existing setitimer() code into unique module, but I had never done it because lack of time, so three slots were reserved for setitimer. I can see that there is inconsistent between TIMER_MAX and maximum timers user can create, your both methods will work. Thanks, David Xu
2010/9/6 Alexander Motin <mav@freebsd.org>: > Hi. > > Sorry, but this is not my area of knowledge yet. I am working on kernel > time event producing, while this is consuming, half way to user-level. > > Still few comments from quick review: > =A0- patch seems slightly incomplete. At least in itimers_alloc() loop > condition need to be changed. Good catch. I'll need to look around for more potential issues with TIMER_M= AX. > =A0- To reduce chance of alike mistakes I would implement that patch in > opposite way, by defining TIMER_MAX as (32 + TIMER_RESERVED), but > reporting to user-level as TIMER_MAX - TIMER_RESERVED. Why? A timer ID is a timer ID... Linux reports whacky values for timer IDs, and while we report fairly sensical ones (0-TIMER_MAX), people need to check the timer ID value, and don't assume that the return value is correct. There is one case that I can think of that you're guarding against (poorly written code, where timer ID =3D=3D 0, someone whacks a kernel timer by accident as root, and things don't go so well). That's one thing that I suppose the random timer ID values in Linux protect against (they aren't really deterministic from what I saw, so people either get things spot on by a stroke of luck, or their app goes south with a segfault, etc). Thanks, -Garrett
2010/9/6 David Xu <davidxu@freebsd.org>: > Alexander Motin wrote: >> >> Hi. >> >> Sorry, but this is not my area of knowledge yet. I am working on kernel >> time event producing, while this is consuming, half way to user-level. >> >> Still few comments from quick review: >> =A0- patch seems slightly incomplete. At least in itimers_alloc() loop >> condition need to be changed. >> =A0- To reduce chance of alike mistakes I would implement that patch in >> opposite way, by defining TIMER_MAX as (32 + TIMER_RESERVED), but >> reporting to user-level as TIMER_MAX - TIMER_RESERVED. >> =A0- I have looked on setitimer() implementation referenced here and fou= nd >> no any interaction with timer_create() API and TIMER_MAX. There is >> indeed many loose ends which make me thing there was or intended to be >> something, but I haven't found it. Have I missed something? >> =A0- this code was written by David Xu. I would asked him. >> > > Yes, the code was written by me long time ago, my original intention > was to not only implement timer_create() but also merge existing > setitimer() code into unique module, but I had never done it > because lack of time, so three slots were reserved for setitimer. > I can see that there is inconsistent between TIMER_MAX and maximum > timers user can create, your both methods will work. Understood. Are there any other spots in the code that we need to be aware of in this particular area which are incomplete (I've seen that you've done a fair amount of work in libc/compat43 and libthr in the signal handling sections as well)? Thanks, -Garrett
Garrett Cooper wrote: > 2010/9/6 Alexander Motin <mav@freebsd.org>: >> Sorry, but this is not my area of knowledge yet. I am working on kernel >> time event producing, while this is consuming, half way to user-level. >> >> Still few comments from quick review: >> - patch seems slightly incomplete. At least in itimers_alloc() loop >> condition need to be changed. > > Good catch. I'll need to look around for more potential issues with TIMER_MAX. > >> - To reduce chance of alike mistakes I would implement that patch in >> opposite way, by defining TIMER_MAX as (32 + TIMER_RESERVED), but >> reporting to user-level as TIMER_MAX - TIMER_RESERVED. > > Why? A timer ID is a timer ID... Linux reports whacky values for timer > IDs, and while we report fairly sensical ones (0-TIMER_MAX), people > need to check the timer ID value, and don't assume that the return > value is correct. There is one case that I can think of that you're > guarding against (poorly written code, where timer ID == 0, someone > whacks a kernel timer by accident as root, and things don't go so > well). That's one thing that I suppose the random timer ID values in > Linux protect against (they aren't really deterministic from what I > saw, so people either get things spot on by a stroke of luck, or their > app goes south with a segfault, etc). While I indeed worried about some possible ID constraints in user-level, I was mostly speaking about kernel code clearness. My point was depending on assumption that from kernel side all usual and reserved timers handled same way and so it would be nice to have dedicated constant defining their count instead of use addition every time, having a constant chance of mistake. This point is surely not mandatory and may be ungrounded, but I would at least added one more constant describing full number of timers, including reserved. Also, as soon as reserved timers are never really used, we could probably choose which one to reserve - first or last 3, if it could make code more clear. -- Alexander Motin
Garrett Cooper wrote: > 2010/9/6 David Xu <davidxu@freebsd.org>: >> Alexander Motin wrote: >>> Hi. >>> >>> Sorry, but this is not my area of knowledge yet. I am working on kernel >>> time event producing, while this is consuming, half way to user-level. >>> >>> Still few comments from quick review: >>> - patch seems slightly incomplete. At least in itimers_alloc() loop >>> condition need to be changed. >>> - To reduce chance of alike mistakes I would implement that patch in >>> opposite way, by defining TIMER_MAX as (32 + TIMER_RESERVED), but >>> reporting to user-level as TIMER_MAX - TIMER_RESERVED. >>> - I have looked on setitimer() implementation referenced here and found >>> no any interaction with timer_create() API and TIMER_MAX. There is >>> indeed many loose ends which make me thing there was or intended to be >>> something, but I haven't found it. Have I missed something? >>> - this code was written by David Xu. I would asked him. >>> >> Yes, the code was written by me long time ago, my original intention >> was to not only implement timer_create() but also merge existing >> setitimer() code into unique module, but I had never done it >> because lack of time, so three slots were reserved for setitimer. >> I can see that there is inconsistent between TIMER_MAX and maximum >> timers user can create, your both methods will work. > > Understood. Are there any other spots in the code that we need to > be aware of in this particular area which are incomplete (I've seen > that you've done a fair amount of work in libc/compat43 and libthr in > the signal handling sections as well)? > Thanks, > -Garrett > Note that there is a userland library called librt which wraps around the kernel system calls, the reason is specification requires SEGEV_THREAD notification type, and struct sigevent contains pthread_attr_t pointer which is used for creating service thread, the pthread is unknown to kernel, so the library is created, translating SIGEV_THREAD_ID into SIGEV_THREAD notification. the timer id is not directly accessed by user code, instead user code uses timer_t which is a pointer of structure __timer, however the timer id is saved in the structure. Regards, David Xu
For bugs matching the following criteria: Status: In Progress Changed: (is less than) 2014-06-01 Reset to default assignee and clear in-progress tags. Mail being skipped
Keyword: patch or patch-ready – in lieu of summary line prefix: [patch] * bulk change for the keyword * summary lines may be edited manually (not in bulk). Keyword descriptions and search interface: <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>