Messages in this thread | | | Date | Mon, 13 Oct 2003 14:09:36 +0200 | From | Andrea Arcangeli <> | Subject | Re: 2.4.x performance tests Re: [PATCH] BUG() in exec_mmap() |
| |
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/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |