Question about locking in mmap.c

Question about locking in mmap.c

[Dave McCracken]

It's been my understanding that most vma manipulation is protected by
mm->mmap_sem, and the page table is protected by mm->page_table_lock.  I've
been rummaging through mmap.c and see a number of places that take
page_table_lock when the code is about to make changes to the vma chains.
These places are already holding mmap_sem for write.

My question is what is page_table_lock supposed to be protecting against?
Am I wrong that mmap_sem is sufficient to protect against concurrent
changes to the vmas?

Dave McCracken

======================================================================
Dave McCracken          IBM Linux Base Kernel Team      1-512-838-3059
dmccr@us.ibm.com                                        T/L   678-3059

--
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-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: Question about locking in mmap.c

[Hugh Dickins]

On Wed, 28 May 2003, Dave McCracken wrote:
> It's been my understanding that most vma manipulation is protected by
> mm->mmap_sem, and the page table is protected by mm->page_table_lock.  I've
> been rummaging through mmap.c and see a number of places that take
> page_table_lock when the code is about to make changes to the vma chains.
> These places are already holding mmap_sem for write.
> 
> My question is what is page_table_lock supposed to be protecting against?
> Am I wrong that mmap_sem is sufficient to protect against concurrent
> changes to the vmas?

mmap_sem does protect those who use it against concurrent changes to the
vmas, but neither swapout (vmscan.c and rmap.c) nor swapoff (swapfile.c)
use it, and they (in a few places) do need such protection.

swapout used to scan from vma to vma, and page_table_lock was important
to protect that linkage; rmap has changed that all around, so it might
(I've not thought) be possible to relax page_table_locking of vma
insertion/removal now.

swapoff still scans from vma to vma as it always did; but it's no longer
holding mmlist_lock across unuse_process, so it might (again, I've not
thought deeper) be possible to down_read mmap_sem there now, allowing
vma insertion/removal not to take page_table_lock.

But swapout still needs to find_vma, page_table_lock is all that's
protecting that linkage, isn't it?

I doubt that vma_merge actually needs page_table_lock where it's just
lowering the start or raising the end of a vma: split_vma doesn't take
it in the complementary case.  But I'm not entirely convinced.

Hugh

--
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-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: Question about locking in mmap.c

[Dave McCracken]

--On Wednesday, May 28, 2003 10:27:52 -0500 Dave McCracken
<dmccr@us.ibm.com> wrote:

> My question is what is page_table_lock supposed to be protecting against?
> Am I wrong that mmap_sem is sufficient to protect against concurrent
> changes to the vmas?

I decided one way to find out was to remove the page_table_lock from mmap.
I discovered one place it protects against is vmtruncate(), so it's
definitely needed as it stands.  I got an oops in zap_page_range() called
from vmtruncate().

Dave McCracken

======================================================================
Dave McCracken          IBM Linux Base Kernel Team      1-512-838-3059
dmccr@us.ibm.com                                        T/L   678-3059

--
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-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: Question about locking in mmap.c

[William Lee Irwin III]

On Wednesday, May 28, 2003 10:27:52 -0500 Dave McCracken wrote:
>> My question is what is page_table_lock supposed to be protecting against?
>> Am I wrong that mmap_sem is sufficient to protect against concurrent
>> changes to the vmas?

On Wed, May 28, 2003 at 05:10:06PM -0500, Dave McCracken wrote:
> I decided one way to find out was to remove the page_table_lock from mmap.
> I discovered one place it protects against is vmtruncate(), so it's
> definitely needed as it stands.  I got an oops in zap_page_range() called
> from vmtruncate().

do_mmap_pgoff() should at least be taking ->i_shared_sem around
__vma_link(). The analogue for do_munmap() is not quite the case;
it appears that the links aren't simultaneously removed under
->i_shared_sem, which would be the cause of it.

vma_merge() appears to be safe, but unmap_vma() does not. The lock
hierarchy is essentially inconsistent around this area, so trylocking
and failure handling would be required to convert it.


-- wli
--
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-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>