Hi Yann, I am coding the fix now and I a question regarding to your comments. I would like to get some clarification to make sure I totally understand.
You mentioned: "with a process pool cleanup registered the first time to free() the whole thing on stop" My question is: if we do malloc and realloc(), I think it would be just in memory. I can free() the whole thing on httpd stop. Do you mean that or you actually mean we need create a new memory pool, allocate/realloc the memory there and register the cleanup on stop? Thanks, Lucy -----Original Message----- From: Lu, Yingqi [mailto:[email protected]] Sent: Monday, October 06, 2014 7:46 AM To: [email protected] Subject: RE: svn commit: r1599531 - in /httpd/httpd/trunk: CHANGES include/ap_listen.h server/listen.c server/mpm/event/event.c server/mpm/prefork/prefork.c server/mpm/worker/worker.c server/mpm_unix.c Hi Yann, Thanks very much for your feedback. I will send another update soon to address the restart issues. Also, inactive CPUs will not be scheduled for httpd. I will change back _SC_NPROCESSORS_CONF to _SC_NPROCESSORS_ONLN. Thanks, Lucy -----Original Message----- From: Yann Ylavic [mailto:[email protected]] Sent: Monday, October 06, 2014 1:12 AM To: httpd Subject: Re: svn commit: r1599531 - in /httpd/httpd/trunk: CHANGES include/ap_listen.h server/listen.c server/mpm/event/event.c server/mpm/prefork/prefork.c server/mpm/worker/worker.c server/mpm_unix.c Hi Yingqi, On Sun, Oct 5, 2014 at 11:36 PM, Lu, Yingqi <[email protected]> wrote: > To address your first comment, the issue with pconf pool is that bucket array > value needs to be retained via restart and graceful restart. Based on your > comments, I now put bucket array into the retained_data struct for all the > mpms. Hope this works. The problem IMHO is that ap_daemons_limit (used to compute the size of the bucket array) may not be constant accross restarts (depending on the new configuration). Maybe you could use a retained bucket array to copy the current values before graceful restart and restore them after in the pconf allocated array (the one really used by the parent process and the new generation of children). To address the memory leak, since the size may change, I think the retained array would have to be malloc()ed instead, and possibly realloc()ed on restarts (cleared when non graceful) if there is not enough space to handle the new generation (with a process pool cleanup registered the first time to free() the whole thing on stop, and make valgrind happy). Also, since the number of listenners (children) needs to remain constant (IIRC, or connections may be reset), maybe you'll have to make sure on graceful restart that the previous generation of children has really stopped listenning before starting new children. Maybe this is always the case already, but the race condition seems more problematic when SO_REUSEPORT is used. > Regarding to your second question, based on previous patch code, num_buckets > is calculated based on the active CPU threads count. I am thinking maybe it > is better to do the calculation based on total number of CPU threads instead. > This keeps num_buckets to be a constant number as long as the system is > running. That is the reason I now change CPU thread count check from > _SC_NPROCESSORS_ONLN to _SC_NPROCESSORS_CONF. I must have missed the point here, will inactive CPUs be scheduled for httpd? Otherwise, I don't see why they should be taken into account for the number of buckets... Regards, Yann.
