From 2d015bd67c8ab20d1b0f9bd1d84a1dc8fd42310f Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Thu, 10 Nov 2005 15:04:06 +0000 Subject: [PATCH] * bsd_helper.cc (securityinit): New function. Move initialization of security related variables from ipcinit here. * bsd_helper.h (securityinit): Add prototype. * cygserver.cc (main): Call securityinit right after wincap.init. * process.cc (process_cache::process): Fix maximum process condition. * README: Add description for new -p/--process-cache option. * bsd_helper.cc (default_tun_check): Add kern.srv.process_cache_size entry to tunable_params. Set max value of kern.srv.request_threads to 310. * cygserver.cc (SERVER_VERSION): Set to 1.20. (print_usage): Print usage of new parameter -p. (main): Add process cache parameter handling. Accomodate new max value of request threads. * cygserver.conf: Add kern.srv.process_cache_size tunable parameter. Accomodate new max value of kern.srv.request_threads. * process.cc: Fix a comment. (process_cache::process_cache): Add max process cache size parameter. Change _cache_add_trigger to manual reset event. (struct pcache_wait_t): New struct used as parameter to pcache_wait_thread. (pcache_wait_thread): New thread function used for threaded process cache. (process_cache::wait_for_processes): Use threaded waiting if number of processes to wait for is bigger than 62. Always check all processes to avoid race under heavy load. (process_cache::sync_wait_array): Remove useless assert. Reset _cache_add_trigger right at the start since it's manual reset now. Accomodate threaded waiting. * process.h (process_cache::process_cache): Add max_procs parameter. (process_cache::_max_process_count): New member. (process_cache::_wait_array: Raise to allow up to 5 wait threads. (process_cache::_process_array): Ditto. --- winsup/cygserver/ChangeLog | 41 +++++++++ winsup/cygserver/README | 21 +++++ winsup/cygserver/bsd_helper.cc | 17 ++-- winsup/cygserver/bsd_helper.h | 2 + winsup/cygserver/cygserver.cc | 30 +++++-- winsup/cygserver/cygserver.conf | 9 +- winsup/cygserver/process.cc | 148 ++++++++++++++++++++++---------- winsup/cygserver/process.h | 9 +- 8 files changed, 213 insertions(+), 64 deletions(-) diff --git a/winsup/cygserver/ChangeLog b/winsup/cygserver/ChangeLog index 0e4eee837..bee49e054 100644 --- a/winsup/cygserver/ChangeLog +++ b/winsup/cygserver/ChangeLog @@ -1,7 +1,48 @@ +2005-11-10 Corinna Vinschen + + * bsd_helper.cc (securityinit): New function. Move initialization + of security related variables from ipcinit here. + * bsd_helper.h (securityinit): Add prototype. + * cygserver.cc (main): Call securityinit right after wincap.init. + 2005-11-10 Corinna Vinschen * bsd_log.cc (_vpanic): LOG_EMERG is overkill, use LOG_CRIT. +2005-11-09 Corinna Vinschen + + * process.cc (process_cache::process): Fix maximum process condition. + +2005-10-24 Corinna Vinschen + + * README: Add description for new -p/--process-cache option. + * bsd_helper.cc (default_tun_check): Add kern.srv.process_cache_size + entry to tunable_params. Set max value of kern.srv.request_threads + to 310. + * cygserver.cc (SERVER_VERSION): Set to 1.20. + (print_usage): Print usage of new parameter -p. + (main): Add process cache parameter handling. Accomodate new max + value of request threads. + * cygserver.conf: Add kern.srv.process_cache_size tunable parameter. + Accomodate new max value of kern.srv.request_threads. + * process.cc: Fix a comment. + (process_cache::process_cache): Add max process cache size parameter. + Change _cache_add_trigger to manual reset event. + (struct pcache_wait_t): New struct used as parameter to + pcache_wait_thread. + (pcache_wait_thread): New thread function used for threaded process + cache. + (process_cache::wait_for_processes): Use threaded waiting if number + of processes to wait for is bigger than 62. Always check all processes + to avoid race under heavy load. + (process_cache::sync_wait_array): Remove useless assert. Reset + _cache_add_trigger right at the start since it's manual reset now. + Accomodate threaded waiting. + * process.h (process_cache::process_cache): Add max_procs parameter. + (process_cache::_max_process_count): New member. + (process_cache::_wait_array: Raise to allow up to 5 wait threads. + (process_cache::_process_array): Ditto. + 2005-08-08 Christopher Faylor * cygserver.cc (main): Call wincap.init() earlier to avoid a NULL diff --git a/winsup/cygserver/README b/winsup/cygserver/README index 209953147..39dcd9f98 100644 --- a/winsup/cygserver/README +++ b/winsup/cygserver/README @@ -51,6 +51,27 @@ Cygserver command line options: under heavy load conditions or on slow machines. Configuration file option: kern.srv.request_threads + -p, --process-cache + + Number of processes which can connect concurrently to cygserver. + Default is 62. Each process connected to cygserver is a synchronization + object which has to be maintained. The data structure to maintain these + processes is the so-called "process cache". In theory, an arbitrary + number of processes could connect to cygserver, but due to the need to + synchronize, the higher the number of connected processes, the more + synchronization overhead exists. By using this option, you can set an + upper limit to the synchronization effort. If more than 62 processes + try to connect to cygserver concurrently, two additional synchronization + threads are necessary, and one for each further 62 concurrent + processes. So, useful values for the --process-cache option are 62, 124, + 186, 248, 310. 310 is the maximum value. + Configuration file option: kern.srv.process_cache_size + + NOTE: The number of child processes of a single parent process is limited + to 256. So in case of taking advantage of a process cache size beyond 256, + keep in mind that not all of these processes can be child processes of one + single parent process. + -d, --debug Log debug messages to stderr. These will clutter your stderr output with diff --git a/winsup/cygserver/bsd_helper.cc b/winsup/cygserver/bsd_helper.cc index 9fb3c9bfa..53ae4a070 100644 --- a/winsup/cygserver/bsd_helper.cc +++ b/winsup/cygserver/bsd_helper.cc @@ -248,6 +248,14 @@ SECURITY_ATTRIBUTES sec_all_nih = { sizeof (SECURITY_ATTRIBUTES), &sec_all_nih_sd, FALSE }; +void +securityinit () +{ + InitializeSecurityDescriptor (&sec_all_nih_sd, SECURITY_DESCRIPTOR_REVISION); + SetSecurityDescriptorDacl (&sec_all_nih_sd, TRUE, 0, FALSE); + init_admin_sid (); +} + /* Global vars, determining whether the IPC stuff should be started or not. */ tun_bool_t support_sharedmem = TUN_UNDEF; tun_bool_t support_msgqueues = TUN_UNDEF; @@ -256,10 +264,6 @@ tun_bool_t support_semaphores = TUN_UNDEF; void ipcinit () { - InitializeSecurityDescriptor (&sec_all_nih_sd, SECURITY_DESCRIPTOR_REVISION); - SetSecurityDescriptorDacl (&sec_all_nih_sd, TRUE, 0, FALSE); - - init_admin_sid (); mtx_init (&Giant, "Giant", NULL, MTX_DEF); msleep_init (); ipcexit_event = CreateEvent (NULL, TRUE, FALSE, NULL); @@ -553,8 +557,9 @@ default_tun_check (tun_struct *that, char *value, const char *fname) static tun_struct tunable_params[] = { /* SRV */ - { "kern.srv.cleanup_threads", TUN_INT, {0}, {1}, {16}, default_tun_check}, - { "kern.srv.request_threads", TUN_INT, {0}, {1}, {64}, default_tun_check}, + { "kern.srv.cleanup_threads", TUN_INT, {0}, {1}, {32}, default_tun_check}, + { "kern.srv.request_threads", TUN_INT, {0}, {1}, {310}, default_tun_check}, + { "kern.srv.process_cache_size", TUN_INT, {0}, {1}, {310}, default_tun_check}, { "kern.srv.sharedmem", TUN_BOOL, {TUN_UNDEF}, {TUN_FALSE}, {TUN_TRUE}, default_tun_check}, { "kern.srv.msgqueues", TUN_BOOL, {TUN_UNDEF}, {TUN_FALSE}, {TUN_TRUE}, default_tun_check}, { "kern.srv.semaphores", TUN_BOOL, {TUN_UNDEF}, {TUN_FALSE}, {TUN_TRUE}, default_tun_check}, diff --git a/winsup/cygserver/bsd_helper.h b/winsup/cygserver/bsd_helper.h index 14849e918..014b82a05 100644 --- a/winsup/cygserver/bsd_helper.h +++ b/winsup/cygserver/bsd_helper.h @@ -37,6 +37,8 @@ extern tun_bool_t support_semaphores; extern SECURITY_ATTRIBUTES sec_all_nih; +void securityinit (void); + int win_copyin (struct thread *, const void *, void *, size_t); int win_copyout (struct thread *, const void *, void *, size_t); #define copyin(a,b,c) win_copyin((td),(a),(b),(c)) diff --git a/winsup/cygserver/cygserver.cc b/winsup/cygserver/cygserver.cc index d19f1755c..a73c5ef95 100644 --- a/winsup/cygserver/cygserver.cc +++ b/winsup/cygserver/cygserver.cc @@ -37,7 +37,7 @@ details. */ #define DEF_CONFIG_FILE "" SYSCONFDIR "/cygserver.conf" -#define SERVER_VERSION "1.12" +#define SERVER_VERSION "1.20" GENERIC_MAPPING access_mapping; @@ -466,6 +466,7 @@ print_usage (const char *const pgm) "\n" "Performance options:\n" " -c, --cleanup-threads Number of cleanup threads to use.\n" +" -p, --process-cache Size of process cache.\n" " -r, --request-threads Number of request threads to use.\n" "\n" "Logging options:\n" @@ -534,6 +535,7 @@ main (const int argc, char *argv[]) {"help", no_argument, NULL, 'h'}, {"log-level", required_argument, NULL, 'l'}, {"no-sharedmem", no_argument, NULL, 'm'}, + {"process-cache", required_argument, NULL, 'p'}, {"no-msgqueues", no_argument, NULL, 'q'}, {"request-threads", required_argument, NULL, 'r'}, {"no-semaphores", no_argument, NULL, 's'}, @@ -544,10 +546,11 @@ main (const int argc, char *argv[]) {0, no_argument, NULL, 0} }; - const char opts[] = "c:deEf:hl:mqr:sSvyY"; + const char opts[] = "c:deEf:hl:mp:qr:sSvyY"; long cleanup_threads = 0; long request_threads = 0; + long process_cache_size = 0; bool shutdown = false; const char *config_file = DEF_CONFIG_FILE; bool force_config_file = false; @@ -568,6 +571,7 @@ main (const int argc, char *argv[]) int opt; wincap.init (); + securityinit (); opterr = 0; while ((opt = getopt_long (argc, argv, opts, longopts, NULL)) != EOF) @@ -576,8 +580,8 @@ main (const int argc, char *argv[]) case 'c': c = NULL; cleanup_threads = strtol (optarg, &c, 10); - if (cleanup_threads <= 0 || cleanup_threads > 16 || (c && *c)) - panic ("Number of cleanup threads must be between 1 and 16"); + if (cleanup_threads <= 0 || cleanup_threads > 32 || (c && *c)) + panic ("Number of cleanup threads must be between 1 and 32"); break; case 'd': @@ -612,6 +616,13 @@ main (const int argc, char *argv[]) support_sharedmem = TUN_FALSE; break; + case 'p': + c = NULL; + process_cache_size = strtol (optarg, &c, 10); + if (process_cache_size <= 0 || process_cache_size > 310 || (c && *c)) + panic ("Size of process cache must be between 1 and 310"); + break; + case 'q': support_msgqueues = TUN_FALSE; break; @@ -619,8 +630,8 @@ main (const int argc, char *argv[]) case 'r': c = NULL; request_threads = strtol (optarg, &c, 10); - if (request_threads <= 0 || request_threads > 64 || (c && *c)) - panic ("Number of request threads must be between 1 and 64"); + if (request_threads <= 0 || request_threads > 310 || (c && *c)) + panic ("Number of request threads must be between 1 and 310"); break; case 's': @@ -688,6 +699,11 @@ main (const int argc, char *argv[]) if (!request_threads) request_threads = 10; + if (!process_cache_size) + TUNABLE_INT_FETCH ("kern.srv.process_cache_size", &process_cache_size); + if (!process_cache_size) + process_cache_size = 62; + if (support_sharedmem == TUN_UNDEF) TUNABLE_BOOL_FETCH ("kern.srv.sharedmem", &support_sharedmem); if (support_sharedmem == TUN_UNDEF) @@ -714,7 +730,7 @@ main (const int argc, char *argv[]) transport_layer_base *const transport = create_server_transport (); assert (transport); - process_cache cache (cleanup_threads); + process_cache cache (process_cache_size, cleanup_threads); server_submission_loop submission_loop (&request_queue, transport, &cache); diff --git a/winsup/cygserver/cygserver.conf b/winsup/cygserver/cygserver.conf index 2e3bc2efb..c224c0e4c 100644 --- a/winsup/cygserver/cygserver.conf +++ b/winsup/cygserver/cygserver.conf @@ -1,4 +1,4 @@ -# cygserver.conf, Copyright(C) 2003 Red Hat Inc. +# cygserver.conf, Copyright(C) 2003, 2005 Red Hat Inc. # # Contains configurable parameters for the cygserver. # @@ -21,9 +21,14 @@ # kern.srv.request_threads: No. of cygserver threads used to serve # application requests. -# Default: 10, Min: 1, Max: 64, command line option -r, --request-threads +# Default: 10, Min: 1, Max: 310, command line option -r, --request-threads #kern.srv.request_threads 10 +# kern.srv.process_cache_size: No. of concurrent processes which can be handled +# by Cygserver concurrently. +# Default: 62, Min: 1, Max: 310, command line option -p, --process-cache +#kern.srv.process_cache_size 62 + # kern.srv.msgqueues: Determines whether XSI Message Queue support should be # started, "yes" (or "true", "y", "t", "1") or "no" (or "false", "n", "f", "0"). # These values are valid for all binary type options. diff --git a/winsup/cygserver/process.cc b/winsup/cygserver/process.cc index eb7e0940d..cc665d88e 100644 --- a/winsup/cygserver/process.cc +++ b/winsup/cygserver/process.cc @@ -88,11 +88,10 @@ process::~process () } /* No need to be thread-safe as this is only ever called by - * process_cache::remove_process (). If it has to be made thread-safe - * later on, it should not use the `access' critical section as that - * is held by the client request handlers for an arbitrary length of - * time, i.e. while they do whatever processing is required for a - * client request. + * process_cache::check_and_remove_process (). If it has to be made + * thread-safe later on, it should not use the `access' critical section as + * that is held by the client request handlers for an arbitrary length of time, + * i.e. while they do whatever processing is required for a client request. */ DWORD process::check_exit_code () @@ -200,10 +199,12 @@ process_cache::submission_loop::request_loop () /*****************************************************************************/ -process_cache::process_cache (const unsigned int initial_workers) +process_cache::process_cache (const size_t max_procs, + const unsigned int initial_workers) : _queue (initial_workers), _submitter (this, &_queue), // true == interruptible _processes_count (0), + _max_process_count (max_procs), _processes_head (NULL), _cache_add_trigger (NULL) { @@ -211,7 +212,7 @@ process_cache::process_cache (const unsigned int initial_workers) InitializeCriticalSection (&_cache_write_access); _cache_add_trigger = CreateEvent (NULL, // SECURITY_ATTRIBUTES - FALSE, // Auto-reset + TRUE, // Manual-reset FALSE, // Initially non-signalled NULL); // Anonymous @@ -251,13 +252,12 @@ process_cache::process (const pid_t cygpid, const DWORD winpid, if (!entry) { - if (_processes_count + SPECIALS_COUNT >= MAXIMUM_WAIT_OBJECTS) + if (_processes_count >= _max_process_count) { LeaveCriticalSection (&_cache_write_access); system_printf (("process limit (%d processes) reached; " "new connection refused for %d(%lu)"), - MAXIMUM_WAIT_OBJECTS - SPECIALS_COUNT, - cygpid, winpid); + _max_process_count, cygpid, winpid); return NULL; } @@ -291,40 +291,93 @@ process_cache::process (const pid_t cygpid, const DWORD winpid, return entry; } +struct pcache_wait_t +{ + size_t index; + size_t count; + HANDLE *hdls; +}; + +static DWORD WINAPI +pcache_wait_thread (const LPVOID param) +{ + pcache_wait_t *p = (pcache_wait_t *) param; + + DWORD rc = WaitForMultipleObjects (p->count, p->hdls, FALSE, INFINITE); + ExitThread (rc == WAIT_FAILED ? rc : rc + p->index); +} + void process_cache::wait_for_processes (const HANDLE interrupt_event) { // Update `_wait_array' with handles of all current processes. + size_t idx; const size_t count = sync_wait_array (interrupt_event); debug_printf ("waiting on %u objects in total (%u processes)", count, _processes_count); - const DWORD rc = WaitForMultipleObjects (count, _wait_array, - FALSE, INFINITE); + DWORD rc = WAIT_FAILED; - if (rc == WAIT_FAILED) + if (count <= 64) { - system_printf ("could not wait on the process handles, error = %lu", - GetLastError ()); - abort (); + /* If count <= 64, a single WaitForMultipleObjects is sufficient and + we can simply wait in the main thread. */ + rc = WaitForMultipleObjects (count, _wait_array, FALSE, INFINITE); + if (rc == WAIT_FAILED) + { + system_printf ("could not wait on the process handles, error = %lu", + GetLastError ()); + abort (); + } + } + else + { + /* If count > 64 we have to create sub-threads which wait for the + actual wait objects and the main thread waits for the termination + of one of the threads. */ + HANDLE main_wait_array[5] = { NULL }; + DWORD mcount = 0; + + for (idx = 0; idx < count; idx += 64) + { + pcache_wait_t p = { idx, min (count - idx, 64), _wait_array + idx }; + main_wait_array[mcount++] = CreateThread (NULL, 0, pcache_wait_thread, + &p, 0, NULL); + } + + rc = WaitForMultipleObjects (mcount, main_wait_array, FALSE, INFINITE); + if (rc == WAIT_FAILED) + { + system_printf ("could not wait on the process handles, error = %lu", + GetLastError ()); + abort (); + } + + /* Check for error condition on signalled sub-thread. */ + GetExitCodeThread (main_wait_array[rc], &rc); + if (rc == WAIT_FAILED) + { + system_printf ("could not wait on the process handles, error = %lu", + GetLastError ()); + abort (); + } + + /* Wake up all waiting threads. _cache_add_trigger gets reset + in sync_wait_array again. */ + SetEvent (_cache_add_trigger); + WaitForMultipleObjects (mcount, main_wait_array, TRUE, INFINITE); + for (idx = 0; idx < mcount; idx++) + CloseHandle (main_wait_array[idx]); } - const size_t start = rc - WAIT_OBJECT_0; - - if (rc < WAIT_OBJECT_0 || start > count) - { - system_printf (("unexpected return code %rc " - "from WaitForMultipleObjects: " - "expected [%u .. %u)"), - rc, WAIT_OBJECT_0, WAIT_OBJECT_0 + count); - abort (); - } - - // Tell all the processes, from the signalled point up, the bad news. - for (size_t index = start; index != count; index++) - if (_process_array[index]) - check_and_remove_process (index); + /* Tell all processes the bad news. This one formerly only checked + processes beginning with the index of the signalled process, but + this can result in processes which are signalled but never removed + under heavy load conditions. */ + for (idx = 0; idx < count; idx++) + if (_process_array[idx]) + check_and_remove_process (idx); } /* @@ -343,12 +396,12 @@ size_t process_cache::sync_wait_array (const HANDLE interrupt_event) { assert (this); - assert (_cache_add_trigger && _cache_add_trigger != INVALID_HANDLE_VALUE); assert (interrupt_event && interrupt_event != INVALID_HANDLE_VALUE); - EnterCriticalSection (&_cache_write_access); + /* Always reset _cache_add_trigger before filling up the array again. */ + ResetEvent (_cache_add_trigger); - assert (_processes_count + SPECIALS_COUNT <= elements (_wait_array)); + EnterCriticalSection (&_cache_write_access); size_t index = 0; @@ -360,19 +413,24 @@ process_cache::sync_wait_array (const HANDLE interrupt_event) _wait_array[index] = ptr->handle (); _process_array[index++] = ptr; - assert (index <= elements (_wait_array)); + if (!ptr->_next || index % 64 == 62) + { + /* Added at the end of each thread's array part for efficiency. */ + _wait_array[index] = interrupt_event; + _process_array[index++] = NULL; + _wait_array[index] = _cache_add_trigger; + _process_array[index++] = NULL; + } } - /* Sorry for shouting, but THESE MUST BE ADDED AT THE END! */ - /* Well, not strictly `must', but it's more efficient if they are :-) */ - - _wait_array[index] = interrupt_event; - _process_array[index++] = NULL; - - _wait_array[index] = _cache_add_trigger; - _process_array[index++] = NULL; - - /* Phew, back to normal volume now. */ + if (!index) + { + /* To get at least *something* to wait for. */ + _wait_array[index] = interrupt_event; + _process_array[index++] = NULL; + _wait_array[index] = _cache_add_trigger; + _process_array[index++] = NULL; + } assert (index <= elements (_wait_array)); diff --git a/winsup/cygserver/process.h b/winsup/cygserver/process.h index 16a5205e9..4702d2818 100644 --- a/winsup/cygserver/process.h +++ b/winsup/cygserver/process.h @@ -1,6 +1,6 @@ /* process.h - Copyright 2001, 2002, 2003, 2004 Red Hat Inc. + Copyright 2001, 2002, 2003, 2004, 2005 Red Hat Inc. Written by Robert Collins @@ -141,7 +141,7 @@ class process_cache friend class submission_loop; public: - process_cache (unsigned int initial_workers); + process_cache (const size_t max_procs, const unsigned int initial_workers); ~process_cache (); class process *process (pid_t cygpid, DWORD winpid, @@ -157,13 +157,14 @@ private: submission_loop _submitter; size_t _processes_count; + size_t _max_process_count; class process *_processes_head; // A list sorted by winpid. // Access to the _wait_array and related fields is not thread-safe, // since they are used solely by wait_for_processes () and its callees. - HANDLE _wait_array[MAXIMUM_WAIT_OBJECTS]; - class process *_process_array[MAXIMUM_WAIT_OBJECTS]; + HANDLE _wait_array[5 * MAXIMUM_WAIT_OBJECTS]; + class process *_process_array[5 * MAXIMUM_WAIT_OBJECTS]; HANDLE _cache_add_trigger; // Actually both add and remove. CRITICAL_SECTION _cache_write_access; // Actually both read and write access.