All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roger Pau Monne <roger.pau@citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Simon Rowe <Simon.Rowe@eu.citrix.com>,
	David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCH] xs: set read_thread stacksize
Date: Thu, 21 Jun 2012 10:39:37 +0100	[thread overview]
Message-ID: <4FE2EBD9.4040101@citrix.com> (raw)
In-Reply-To: <1340270320.21872.31.camel@zakaz.uk.xensource.com>

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.

  reply	other threads:[~2012-06-21  9:39 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-29 15:56 [PATCH] xs: set read_thread stacksize Simon Rowe
2012-05-29 16:39 ` David Vrabel
2012-05-29 19:39   ` Ian Campbell
2012-05-30  7:56     ` Simon Rowe
2012-05-30  9:40       ` Ian Campbell
2012-05-30 12:10         ` Simon Rowe
2012-05-31  7:32           ` Ian Campbell
2012-06-21  9:09             ` Roger Pau Monne
2012-06-21  9:18               ` Ian Campbell
2012-06-21  9:39                 ` Roger Pau Monne [this message]
2012-06-21 10:18                   ` Ian Campbell
2012-06-21 10:27                     ` Roger Pau Monne
2012-06-21 10:32                       ` Ian Campbell
2012-06-21 17:19               ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FE2EBD9.4040101@citrix.com \
    --to=roger.pau@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=Simon.Rowe@eu.citrix.com \
    --cc=david.vrabel@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.