[PATCH 2/2] um: allocate a guard page to helper threads

[Johannes Berg]

On Sun, 2020-12-06 at 08:23 +0000, Anton Ivanov wrote:
> On 06/12/2020 08:11, Johannes Berg wrote:
> > On Sat, 2020-12-05 at 22:09 +0000, Anton Ivanov wrote:
> > 
> > > > -unsigned long alloc_stack(int order, int atomic)
> > > > +unsigned long alloc_stack(int atomic)
> > > >    {
> > > > -	unsigned long page;
> > > > +	unsigned long addr;
> > > >    	gfp_t flags = GFP_KERNEL;
> > > >    
> > > >    	if (atomic)
> > > >    		flags = GFP_ATOMIC;
> > > > -	page = __get_free_pages(flags, order);
> > > > +	addr = __get_free_pages(flags, 1);
> > > Why are you dropping order?
> > It was always called with 0 as the order argument, so I didn't think it
> > was relevant. I guess we can keep it and allocate order+1, and then
> > reserve a page, but if then e.g. you wanted order 2 (16k) you'd actually
> > get 28k of usable stack and use up 32k. So I thought it was less
> > surprising if we'd remove it.
> > 
> > > I believe we do not fit in a stack of order
> > > less than 2 on newer 64 bit and we need even more with valgrind.
> > Not sure what you mean by this, tbh. Running the whole of UML in
> > valgrind? Why would that require more stack?
> 
> According to the description of the stack order option in the config it 
> does.

Ah ok, you're referring to CONFIG_KERNEL_STACK_ORDER. That indeed
affects only *kernel* threads, not the helper threads that are allocated
here.

Not sure why valgrind would matter, but perhaps it does some magic with
stacks under the hood, don't really know well what it really does.

> > Kernel threads are
> > unaffected, and the helper threads don't do much that would really make
> > a significant difference between 32 and 64 bit?
> 
> OK, I need to have a look again on where do we use the order parameter 
> and what that config option really means.

Well, it affects THREAD_SIZE_ORDER, which is used in kernel/fork.c to
allocate the kernel stacks for new threads, so that makes sense.

But the code I changed here has no relation to it, it's just for the
userspace helper threads such as winch_thread :)

johannes