Hello,
On Fri, Oct 13, 2000 at 01:20:23AM +0100, davej@suse.de wrote:
> > 9. To Do
> > * mm->rss is modified in some places without holding the
> > page_table_lock (sct)
>
> Any of the mm gurus give the patch below a quick once over ?
> Is this adequate, or is there more to this than the description implies?
The patch is basically ok, except one point.
[snip]
> diff -urN --exclude-from=/home/davej/.exclude linux/mm/vmscan.c linux.dj/mm/vmscan.c
> --- linux/mm/vmscan.c Mon Oct 2 20:02:20 2000
> +++ linux.dj/mm/vmscan.c Wed Oct 11 23:46:01 2000
> @@ -102,7 +102,9 @@
> set_pte(page_table, swp_entry_to_pte(entry));
> drop_pte:
> UnlockPage(page);
> + spin_lock (&mm->page_table_lock);
> mm->rss--;
> + spin_unlock (&mm->page_table_lock);
> flush_tlb_page(vma, address);
> deactivate_page(page);
> page_cache_release(page);
> @@ -170,7 +172,9 @@
> struct file *file = vma->vm_file;
> if (file) get_file(file);
> pte_clear(page_table);
> + spin_lock (&mm->page_table_lock);
> mm->rss--;
> + spin_unlock (&mm->page_table_lock);
> flush_tlb_page(vma, address);
> vmlist_access_unlock(mm);
> error = swapout(page, file);
> @@ -202,7 +206,9 @@
> add_to_swap_cache(page, entry);
>
> /* Put the swap entry into the pte after the page is in swapcache */
> + spin_lock (&mm->page_table_lock);
> mm->rss--;
> + spin_unlock (&mm->page_table_lock);
> set_pte(page_table, swp_entry_to_pte(entry));
> flush_tlb_page(vma, address);
> vmlist_access_unlock(mm);
page_table_lock is supposed to protect normal page table activity (like
what's done in page fault handler) from swapping out.
However, grabbing this lock in swap-out code is completely missing!
In vmscan.c the question is not only about rss protection, but about real
protection for page table entries.
It may be something like
--- mm/vmscan.c.ptl Fri Oct 13 12:09:51 2000
+++ mm/vmscan.c Fri Oct 13 12:19:10 2000
@@ -150,6 +150,7 @@
if (file) get_file(file);
pte_clear(page_table);
vma->vm_mm->rss--;
+ spin_unlock(&mm->page_table_lock);
flush_tlb_page(vma, address);
vmlist_access_unlock(vma->vm_mm);
error = swapout(page, file);
@@ -182,6 +183,7 @@
/* Put the swap entry into the pte after the page is in swapcache */
vma->vm_mm->rss--;
set_pte(page_table, swp_entry_to_pte(entry));
+ spin_unlock(&mm->page_table_lock);
flush_tlb_page(vma, address);
vmlist_access_unlock(vma->vm_mm);
@@ -324,6 +326,7 @@
if (address < vma->vm_start)
address = vma->vm_start;
+ spin_lock(&mm->page_table_lock);
for (;;) {
int result = swap_out_vma(mm, vma, address, gfp_mask);
if (result)
@@ -333,6 +336,7 @@
break;
address = vma->vm_start;
}
+ spin_unlock(&mm->page_table_lock);
}
vmlist_access_unlock(mm);
On Thu, Oct 12, 2000 at 05:29:39PM -0700, David S. Miller wrote:
> From: davej@suse.de
> Date: Fri, 13 Oct 2000 01:20:23 +0100 (BST)
>
> Any of the mm gurus give the patch below a quick once over ? Is
> this adequate, or is there more to this than the description
> implies?
>
> It might make more sense to just make rss an atomic_t.
In most cases where rss is updated page_table_lock is already held.
Best regards
Andrey
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sun Oct 15 2000 - 21:00:24 EST