Re: 3rd version of R/W mmap_sem patch available

[Linus Torvalds <torvalds@transmeta.com>]

On Wed, 21 Mar 2001, Andrew Morton wrote:
>
> I stared long and hard at expand_stack().  Its first access
> to vma->vm_start appears to be safe wrt other threads which
> can alter this, but perhaps the page_table_lock should be
> acquired earlier here?

Hmm.. Probably.

> We now have:
>
> 	free_pgd_slow();
> 	pmd_free_slow();
> 	pte_free_slow();
>
> Could we please have consistent naming back?

Yes, I want to rename free_pgd_slow() to match the others.

> in do_wp_page():
>
>         spin_unlock(&mm->page_table_lock);
>         new_page = alloc_page(GFP_HIGHUSER);
>         if (!new_page)
>                 return -1;
>         spin_lock(&mm->page_table_lock);
>
> Should retake the spinlock before returning.

Thanks, done.

> General comment: an expensive part of a pagefault
> is zeroing the new page.  It'd be nice if we could
> drop the page_table_lock while doing the clear_user_page()
> and, if possible, copy_user_page() functions.  Very nice.

I don't think it's worth it. We should have basically zero contention on
this lock now, and adding complexity to try to release it sounds like a
bad idea when the only way to make contention on it is (a) kswapd (only
when paging stuff out) and (b) multiple threads (only when taking
concurrent page faults).

So I don't really see the point of bothering.

> read_zero_pagealigned()->zap_page_range()
>
> 	The handling of mm->rss is racy.  But I think
> 	it always has been?

It always has been. Right now I think we hold the page_table_lock over
most of them, that the old patch to fix this might end up being just that
one place. Somebody interested in checking?

> This comment in mprotect.c:
> +       /* XXX: maybe this could be down_read ??? - Rik */
>
> I don't think so.  The decisions about where in the
> vma tree to place the new vma would be unprotected and racy.

I think we could potentially find it useful in places to have a

	down_write_start();

	down_write_commit();

in a few places, where "down_write_start()" only guarantees exclusion of
other writers (and write-startes), while down_write_commit() waits for all
the readers to go away.

		Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux.eu.org/Linux-MM/