Re: 2.4.x performance tests Re: [PATCH] BUG() in exec_mmap()

[Andrea Arcangeli <andrea@suse.de>]

On Mon, Oct 13, 2003 at 02:52:44AM -0400, Ernie Petrides wrote:
> --- linux-2.4.21/fs/exec.c.orig
> +++ linux-2.4.21/fs/exec.c
> @@ -452,9 +452,11 @@ static int exec_mmap(void)
>  
>  	old_mm = current->mm;
>  	if (old_mm && atomic_read(&old_mm->mm_users) == 1) {
> +		down_write(&old_mm->mmap_sem);
>  		mm_release();
>  		exit_aio(old_mm);
>  		exit_mmap(old_mm);
> +		up_write(&old_mm->mmap_sem);
>  		return 0;
>  	}

Is there any special reason you take it around mm_release and exit_aio
too? I don't feel this is needed. exit_aio btw still assumes nobody can
race, so it doesn't take any spinlock (brlocks actually) to guard
against other aio threads, I believe that's ok since as worse the other
tasks can mangle the vm with ptrace, they'll never get to mess with aio,
only the current task can and the mm_user == 1 check guarantees we've no
sibiling threads. the mmap_sem shouldn't help exit_aio anyways, if
something it'll make it deadlock if there's any access to the VM that
generates a page fault in the cancel() callback.

So I suggest this sequence should be safe:

	mm_release();
	exit_aio(old_mm);

	down_write(&old_mm->mmap_sem);
	exit_mmap(old_mm);
	up_write(&old_mm->mmap_sem);

Please double check ;)

Andrea - If you prefer relying on open source software, check these links:
	    rsync.kernel.org::pub/scm/linux/kernel/bkcvs/linux-2.[45]/
	    http://www.cobite.com/cvsps/