Summary:	ASTERISK-05150: [patch] Asterisk -p command line option doesn't work on Linux
Reporter:	knielsen (knielsen)	Labels:
Date Opened:	2005-09-26 04:35:59	Date Closed:	2008-01-15 15:49:43.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) pthread_attr_init-patch1-dist.txt ( 1) pthread_attr_init-patch2-dist.txt ( 2) pthread_attr_init-patch3-dist.txt
Description:	Asterisk may be started with the -p option to make it run with higher priority, in an attempt to avoid audio glitches etc. caused by non-Asterisk system activity. For Linux, the code attemps to set real-time scheduling policy. But that doesn't actually work. The reason is that pthreads by default sets the priority for new threads to non-realtime. To inherit the realtime priority for new threads, it is necessary to do pthread_attr_setinheritsched(attr, PTHREAD_INHERIT_SCHED) and pass that attr to pthread_create(). The result is that if you run Asterisk with -p on Linux, the main thread will run with real-time priority, but all other threads will run at normal, non-realtime priority ****** ADDITIONAL INFORMATION ****** Attached patch fixes the problem by introducing ast_pthread_attr_init() and making all Asterisk code use that for thread creation. I used #ifdef __linux__ since the realtime priority stuff for -p is also #ifdef __linux__. But another argument would be that it is cleaner to have our own ast_pthread_attr_init() for all architectures (even if it is currently only needed on Linux), and I would be willing to modify the patch to do this if desired. Disclaimer is on file.
Comments:	By: Kevin P. Fleming (kpfleming) 2005-09-26 11:55:40 This is a good first step, but it will need some minor changes: 1) Ensure that pthread_attr_init is not a macro, then define it as a macro that expands into '__dont_use_pthread_attr_init_use_ast_pthread_attr_init'... see the other replacement functions for pthread stuff for an example. 2) Move the additions for utils.h into the main 'pthreads' related area of that header file. 3) While you're in there... I've wondered (rather often) if we really need to be creating all these pthread_attr structs all over the place, or if we could just centrally allocate a few with 'common' combinations of attributes and then re-use them. The vast majority of threads that are created use the same attribute settings, so this would simplify that code. Give that some thought, and testing if you wish... it would be a nice enhancement. By: Mark Spencer (markster) 2005-09-26 23:27:13 It would seem to be easier just to put this in ast_pthread_create, perhaps?  Then it's a very short change... By: knielsen (knielsen) 2005-09-27 02:28:36 markster: I like it. It does mean that callers will no longer be able to set a different priority for their new thread directly in the pthread_attr_t argument to ast_pthread_create(). Instead they must use pthread_setschedparam() on the new thread after creation. I have added a comment to that effect in the code. Attached is a new patch pthread_attr_init-patch2-dist.txt that implements this and replaces the earlier patch. By: knielsen (knielsen) 2005-09-27 04:35:32 Sigh, there was a typo in that patch which causes a compiler warning. This is fixed in pthread_attr_init-patch3-dist.txt, which replaces the two earlier patches. By: Kevin P. Fleming (kpfleming) 2005-09-29 00:11:53 Committed to CVS HEAD, thanks! By: Digium Subversion (svnbot) 2008-01-15 15:49:43.000-0600 Repository: asterisk Revision: 6692 U   trunk/utils.c ------------------------------------------------------------------------ r6692 | kpfleming | 2008-01-15 15:49:43 -0600 (Tue, 15 Jan 2008) | 2 lines ensure scheduling priority is inherited into new threads (issue ASTERISK-5150) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=6692