From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Pau Monne Subject: Re: [PATCH] xs: set read_thread stacksize Date: Thu, 21 Jun 2012 10:39:37 +0100 Message-ID: <4FE2EBD9.4040101@citrix.com> References: <0cf61ed6ce86de2b61db.1338307000@drall.uk.xensource.com> <201205300856.15605.simon.rowe@eu.citrix.com> <1338370815.31698.26.camel@zakaz.uk.xensource.com> <201205301310.09243.simon.rowe@eu.citrix.com> <1338449525.7864.14.camel@zakaz.uk.xensource.com> <4FE2E4DC.5030500@citrix.com> <1340270320.21872.31.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1340270320.21872.31.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "xen-devel@lists.xen.org" , Ian Jackson , Simon Rowe , David Vrabel List-Id: xen-devel@lists.xenproject.org Ian Campbell wrote: > On Thu, 2012-06-21 at 10:09 +0100, Roger Pau Monne wrote: >> Ian Campbell wrote: >>> On Wed, 2012-05-30 at 13:10 +0100, Simon Rowe wrote: >>>> On Wednesday 30 May 2012 10:40:15 Ian Campbell wrote: >>>> >>>>> If this little trick applies to both NetBSD and uclibc too then I guess >>>>> it would be OK, otherwise I think autoconf is necessary. >>>> It doesn't look to my untrained eye that xenstore is autoconf-aware. The >>>> makefile unilaterally sets USE_PTHREAD for example. >>> It has autoconf stuff available to it, I think, it just hasn't had cause >>> to use it yet... >>> >>> (USE_PTHREAD is a bit of an odd one anyway, it refers only to the client >>> library/cmdline tools and is for building against libc's which don't >>> have pthreads) >>> >>>> Shall I just drop this test for now and if/when xenstore is updated to use >>>> autoconf it can be addressed then? >>> I'd like to here from Roger about what this means for NetBSD and uclibc, >>> if it works on those then I think it is fine to do this. >> Sorry for the delay, I just received this today (don't know why). > > It seemed to have been stuck in my outbox, I thought it was another > unrelated mail (which I sent this morning) so I hit go. > > BTW, I think this patch went in already... > >> I've >> been looking at NetBSD and uClibc, and both have pthread_attr_setstacksize. >> >> What I don't really like is the hardcoded (16 * 1024) value, how do you >> know this is greater than PTHREAD_STACK_MIN? > > pthread_attr_setstacksize(1) specifically says that PTHREAD_STACK_MIN is > 16K, but I don't know if that is a real specified thing or just Linux > bias in the manpage. PTHREAD_STACK_MIN it's also present on NetBSD and uClibc, so I guess we should use PTHREAD_STACK_MIN (or PTHREAD_STACK_MIN * 2) if the default stack size has to be changed to some sensible value (which I still don't think it has to). Can we guarantee PTHREAD_STACK_MIN is not going to change to something greater than 16k thus breaking this patch? > http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_attr_setstacksize.html doesn't seem to say anything about the value of PTHREAD_STACK_MIN. > > How about we fix this when we come across a system which has a smaller > minimum stack? > >> Frankly I don't understand why do we have to touch this, even if you >> requested 256MB of stack it won't we allocated until you get a page >> fault, so you are only using the physical memory you need. > > With 256M stack 4 threads would take up 1G of your virtual address space > (regardless of whether it is populated or not), and 12 threads would > take up 3G -- which is the whole virtual address space of a 32 bit > process, which is rather limiting. This should be set by the OS to a sensible value that allows creating a reasonable number of threads, given that the default in Linux is 8MB, it will allow you to create 375 threads on a 32bit system, which looks like a pretty high number to me. Anyway limiting the stack size of a single thread won't make much of difference regarding this, because all the other threads will still take the default value.