2003-06-04 22:17:18

by Dave McCracken

[permalink] [raw]
Subject: [PATCH] Remove page_table_lock from vma manipulations


After more careful consideration, I don't see any reasons why
page_table_lock is necessary for dealing with vmas. I found one spot in
swapoff, but it was easily changed to mmap_sem. I've beat on this code and
mjb has beat on this code with no problems. Here's the patch to remove it.

Feel free to poke holes in it.

Dave McCracken

======================================================================
Dave McCracken IBM Linux Base Kernel Team 1-512-838-3059
[email protected] T/L 678-3059


Attachments:
(No filename) (550.00 B)
nolock-2.5.70-mm4-1.diff (4.46 kB)
Download all attachments

2003-06-04 22:24:38

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [PATCH] Remove page_table_lock from vma manipulations

On Wed, Jun 04, 2003 at 05:30:25PM -0500, Dave McCracken wrote:
> After more careful consideration, I don't see any reasons why
> page_table_lock is necessary for dealing with vmas. I found one spot in
> swapoff, but it was easily changed to mmap_sem. I've beat on this code and
> mjb has beat on this code with no problems. Here's the patch to remove it.
> Feel free to poke holes in it.

shrink_list() calls try_to_unmap() under pte_chain_lock(page), and
hence try_to_unmap() cannot sleep. Furthermore try_to_unmap() calls
find_vma() under the sole protection of
spin_trylock(&mm->page_table_lock), which I don't see changed to a
read_trylock(&mm->mmap_sem) here.

Hence, this is racy.


-- wli

2003-06-04 22:24:41

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Remove page_table_lock from vma manipulations

Dave McCracken wrote:
>
> After more careful consideration, I don't see any reasons why
> page_table_lock is necessary for dealing with vmas. I found one spot in
> swapoff, but it was easily changed to mmap_sem.

What keeps the VMA tree consistent when try_to_unmap_one()
runs find_vma()?

2003-06-04 22:45:18

by Dave McCracken

[permalink] [raw]
Subject: Re: [PATCH] Remove page_table_lock from vma manipulations


--On Wednesday, June 04, 2003 15:38:09 -0700 Andrew Morton <[email protected]>
wrote:

>> After more careful consideration, I don't see any reasons why
>> page_table_lock is necessary for dealing with vmas. I found one spot in
>> swapoff, but it was easily changed to mmap_sem.
>
> What keeps the VMA tree consistent when try_to_unmap_one()
> runs find_vma()?

Gah. I don't know how I convinced myself that code was safe. It's easily
fixed. How does this one look?

Dave

======================================================================
Dave McCracken IBM Linux Base Kernel Team 1-512-838-3059
[email protected] T/L 678-3059


Attachments:
(No filename) (688.00 B)
nolock-2.5.70-mm4-2.diff (5.34 kB)
Download all attachments

2003-06-05 15:56:25

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] Remove page_table_lock from vma manipulations

On Wed, 4 Jun 2003, Dave McCracken wrote:
>
> Gah. I don't know how I convinced myself that code was safe. It's easily
> fixed. How does this one look?

I think you have to keep page_table_lock in expand_stack (GROWSUP and
down versions) because that is called with only down_read on mmap_sem,
so two could be racing: nothing else protects the various vma field
adjustments there. Otherwise appears correct to me. Beneficial?

Hugh