Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Dec 2017 07:09:10 -0800 (PST)
From:      "Rodney W. Grimes" <freebsd@pdx.rh.CN85.dnsmgr.net>
To:        Conrad Meyer <cem@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r326758 - in head/sys/i386: conf include
Message-ID:  <201712111509.vBBF9Aha081695@pdx.rh.CN85.dnsmgr.net>
In-Reply-To: <201712110432.vBB4WbnE021090@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
[ Charset UTF-8 unsupported, converting... ]
> Author: cem
> Date: Mon Dec 11 04:32:37 2017
> New Revision: 326758
> URL: https://svnweb.freebsd.org/changeset/base/326758
> 
> Log:
>   i386: Bump KSTACK_PAGES default to match amd64
>   
>   Logically, extend r286288 to cover all threads, by default.
>   
>   The world has largely moved on from i386.  Most FreeBSD users and developers

THis is a mistake, there is a huge worled of i386 deployment, not all
the world needs, or even wants amd64, especially in teh virtualization
world when you are running anything with less than 4G of memory, which
I would argue is a huge depolyement base.

64bit code is simply wasteful on these platforms, and though maybe
software developers are ok with that waste, (smart) business people
are not.

>   test on amd64 hardware.  For better or worse, we have written a non-trivial
>   amount of kernel code that relies on stacks larger than 8 kB, and it "just
>   works" on amd64, so there has been little incentive to shrink it.
>   
>   amd64 had its KSTACK_PAGES bumped to 4 back in Peter's initial AMD64 commit,
>   r114349, in 2003.  Since that time, i386 has limped along on a stack half
>   the size.  We've even observed the stack overflows years ago, but neglected
>   to fix the issue; see the 20121223 and 20150728 entries in UPDATING.

This makes perfect sense to have sizeofstackamd64 == 2 * sizofstacki386 if
you think about it for very long, most of what should be on your stack are
pointers and ints, the sizedof pointers/int is 2x on amd64.   If we are
storing lots of none pointer stuff that is very large on the stack something
is probably designed wrong.

Does it makes since to change the KSTACK_PAGES value to be more MI
in how it is defined (I am not saying move it to MI code) so a
better understanding can be had, as in:

	KSTACK_PAGES=(sizeof(void *)/2)

The comment that should go with this is left as an excersize for
the commiter.

The current comment about a pcb, I thought that code was changed
so we only put the pointer to a pcb on the stack.

>   
>   If anyone is concerned with this change, I suggest they configure their
>   AMD64 kernels with KSTACK_PAGES 2 and fix the fallout there first.  Eugene
>   has identified a list of high stack usage functions in the first PR below.

Um, that is the wrong approach for the reason stated above, you would end
up pessimizing on amd64.  Thought getting amd64 to run in 2 pages of stack
would defanitly allow the i386 to run in 2 pages.


>   PR:		219476, 224218
>   Reported by:	eugen@, Shreesh Holla <hshreesh AT yahoo.com>
>   Relnotes:	maybe
>   Sponsored by:	Dell EMC Isilon
> 
> Modified:
>   head/sys/i386/conf/NOTES
>   head/sys/i386/include/param.h
> 
> Modified: head/sys/i386/conf/NOTES
> ==============================================================================
> --- head/sys/i386/conf/NOTES	Mon Dec 11 02:44:15 2017	(r326757)
> +++ head/sys/i386/conf/NOTES	Mon Dec 11 04:32:37 2017	(r326758)
> @@ -968,7 +968,7 @@ device		ndis
>  # KSTACK_PAGES is the number of memory pages to assign to the kernel
>  # stack of each thread.
>  
> -options 	KSTACK_PAGES=3
> +options 	KSTACK_PAGES=5
>  
>  # Enable detailed accounting by the PV entry allocator.
>  
> 
> Modified: head/sys/i386/include/param.h
> ==============================================================================
> --- head/sys/i386/include/param.h	Mon Dec 11 02:44:15 2017	(r326757)
> +++ head/sys/i386/include/param.h	Mon Dec 11 04:32:37 2017	(r326758)
> @@ -113,7 +113,7 @@
>  #define IOPAGES	2		/* pages of i/o permission bitmap */
>  
>  #ifndef KSTACK_PAGES
> -#define KSTACK_PAGES 2		/* Includes pcb! */
> +#define KSTACK_PAGES 4		/* Includes pcb! */
>  #endif
>  #define KSTACK_GUARD_PAGES 1	/* pages of kstack guard; 0 disables */
>  #if KSTACK_PAGES < 4
> 
> 

-- 
Rod Grimes                                                 rgrimes@freebsd.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201712111509.vBBF9Aha081695>