Re: Split 'flush_old_exec' into two functions - 221af7f87b97431e3ee21ce4b0e77d5411cf1549

["H. Peter Anvin" <hpa@zytor.com>]

On 02/02/2010 07:50 AM, Linus Torvalds wrote:
> 
> Normally, that would happen, but this patch got applied early _literally_ 
> because I wanted it to hit -rc6 rather than wait any longer. So it had 
> only a day or two of discussion, and probably just a few hours from the 
> final version.
> 
> That said, I think I may have found the cause.
> 
> Peter: look at setup_new_exec(), and realize that it got moved _down_ to 
> after all the personality setting. So far, so good, that was the 
> intention, but look at what it does:
> 
> 	current->flags &= ~PF_RANDOMIZE;
> 
> and look at how fs/binfmt_elf.c does it not just after the personality 
> setting, but also after
> 
> 	if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space)
> 		current->flags |= PF_RANDOMIZE;
> 
> so it looks like you may have moved it down too much.
> 

Yes, not entirely surprising.

> I think you did that because you wanted to do that
> 
> 	arch_pick_mmap_layout(current->mm);
> 
> in setup_new_exec(). Which makes total sense, but it all means that the 
> whole preparatory patch did way more than my initial one (which put 
> setup_new_exec() right after flush_old_exec())

Yes, that was the intention, and I did specify that I had folded in my
(previously posted as a separate patch) changes; the intent was to avoid
a bisect hole.  I didn't describe it well because of the rush, though.

> Michael, mind trying this (UNTESTED!) patch? It makes conceptual sense, 
> and moves some more of the flushing of the old process state up to 
> "flush_old_exec()" rather than doing it late in "setup_new_exec()".
>
> (I suspect we should also move the signal/fd flushing there, but I doubt 
> it matters)

Quite.

	-hpa