install_page() lockup

install_page() lockup

[Andrew Morton]

I'm getting lockups in install_page() with shared pagetables
enabled.  I haven't really delved into it.  It happens under
heavy memory pressure on SMP.

Ingo's new patch is using install_page much more than we
used to (I don't think I've ever run it before), so we're
running fairly untested codepaths here.

I tried this:

 mm/fremap.c |    2 ++
 1 files changed, 2 insertions(+)

--- 25/mm/fremap.c~a	Tue Oct 22 22:08:26 2002
+++ 25-akpm/mm/fremap.c	Tue Oct 22 22:09:02 2002
@@ -72,7 +72,9 @@ int install_page(struct mm_struct *mm, s
 		pte_page_lock(ptepage);
 		if (page_count(ptepage) > 1) {
 			pte = pte_unshare(mm, pmd, addr);
+			pte_page_unlock(ptepage);
 			ptepage = pmd_page(*pmd);
+			pte_page_lock(ptepage);
 		} else
 			pte = pte_offset_map(pmd, addr);
 	} else {

.

Because doing a pte_page_lock(ptepage) and then losing
track of the page we just locked looks fishy.  Didn't
help though.

Dave could you please review the code in there?  It's probably
something simple.

Thanks.
--
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/

Re: install_page() lockup

[Ingo Molnar]

On Tue, 22 Oct 2002, Andrew Morton wrote:

> Ingo's new patch is using install_page much more than we used to (I
> don't think I've ever run it before), so we're running fairly untested
> codepaths here.

i added install_page() for fremap()'s purposes so i'd be surprised if
anything else used it. I have shared-pte turned off in my tests, will try
with it on as well.

	Ingo

--
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/

Re: install_page() lockup

[Dave McCracken]

--On Wednesday, October 23, 2002 08:49:11 +0200 Ingo Molnar <mingo@elte.hu>
wrote:

> i added install_page() for fremap()'s purposes so i'd be surprised if
> anything else used it. I have shared-pte turned off in my tests, will try
> with it on as well.

As I sent in an earlier mail, I found the bug.  do_file_page needs to
unlock the pte_page_lock, not the page_table_lock.

If I understand the use of install_page correctly, I don't see a reason why
it should be calling pte_unshare.  If it's only installing pages from
existing shared regions it should leave the pte page shared.  The only
reason to call pte_unshare is if the vma for that mm has changed, making
the sharing decision invalid.  Am I missing how this is being used?

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/

Re: install_page() lockup

[Dave McCracken]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]


--On Tuesday, October 22, 2002 22:37:10 -0700 Andrew Morton
<akpm@digeo.com> wrote:

> 
> I'm getting lockups in install_page() with shared pagetables
> enabled.  I haven't really delved into it.  It happens under
> heavy memory pressure on SMP.
> 
> Ingo's new patch is using install_page much more than we
> used to (I don't think I've ever run it before), so we're
> running fairly untested codepaths here.

As Ingo said, he added install_page.

> I tried this:
> 
> (snip)
> 
> Because doing a pte_page_lock(ptepage) and then losing
> track of the page we just locked looks fishy.  Didn't
> help though.

The code was correct.  pte_unshare moves the lock to the new pte page if it
installs one.  I know that's not real clean, but it eliminates multiple
unlock/relock sequences.

> Dave could you please review the code in there?  It's probably
> something simple.

I found the problem.  In memory.c:do_file_page it unlocks the
page_table_lock.  In the new locking scheme, it's actually the
pte_page_lock that's held instead.

The patch to fix this is attached.

Dave McCracken

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

[-- Attachment #2: shpte-2.5.44-mm3-1.diff --]
[-- Type: text/plain, Size: 656 bytes --]

--- 2.5.44-mm3/mm/memory.c	2002-10-23 10:20:08.000000000 -0500
+++ 2.5.44-mm3-shsent/mm/memory.c	2002-10-23 10:42:56.000000000 -0500
@@ -1823,6 +1823,7 @@
 static int do_file_page(struct mm_struct * mm, struct vm_area_struct * vma,
 	unsigned long address, int write_access, pte_t *pte, pmd_t *pmd)
 {
+	struct page *ptepage = pmd_page(*pmd);
 	unsigned long pgoff;
 	int err;
 
@@ -1840,7 +1841,7 @@
 	pgoff = pte_to_pgoff(*pte);
 
 	pte_unmap(pte);
-	spin_unlock(&mm->page_table_lock);
+	pte_page_unlock(ptepage);
 
 	err = vma->vm_ops->populate(vma, address & PAGE_MASK, PAGE_SIZE, vma->vm_page_prot, pgoff, 0);
 	if (err == -ENOMEM)