mremap() pte allocation atomicity error

mremap() pte allocation atomicity error

[William Lee Irwin III]

I'm working on something else atm.

 [<c01187b3>]__might_sleep+0x43/0x47
 [<c013b6d4>]__alloc_pages+0x24/0x20c
 [<c0133650>]file_read_actor+0x0/0x1b0
 [<c01131ed>]pte_alloc_one+0x41/0x104 
 [<c012d05d>]pte_alloc_map+0x4d/0x210
 [<c013bc73>]get_page_cache_size+0xf/0x18
 [<c0135f38>]move_one_page+0xe8/0x328    
 [<c0136061>]move_one_page+0x211/0x328
 [<c0130644>]vm_enough_memory+0x34/0xc0
 [<c01361a9>]move_page_tables+0x31/0x7c
 [<c0136860>]do_mremap+0x66c/0x7ec     
 [<c0136a30>]sys_mremap+0x50/0x73 
 [<c010748f>]syscall_call+0x7/0xb

--
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: mremap() pte allocation atomicity error

[Andrew Morton]

William Lee Irwin III wrote:
> 
> I'm working on something else atm.
> 
>  [<c01187b3>]__might_sleep+0x43/0x47
>  [<c013b6d4>]__alloc_pages+0x24/0x20c
>  [<c0133650>]file_read_actor+0x0/0x1b0
>  [<c01131ed>]pte_alloc_one+0x41/0x104
>  [<c012d05d>]pte_alloc_map+0x4d/0x210
>  [<c013bc73>]get_page_cache_size+0xf/0x18
>  [<c0135f38>]move_one_page+0xe8/0x328
>  [<c0136061>]move_one_page+0x211/0x328
>  [<c0130644>]vm_enough_memory+0x34/0xc0
>  [<c01361a9>]move_page_tables+0x31/0x7c
>  [<c0136860>]do_mremap+0x66c/0x7ec
>  [<c0136a30>]sys_mremap+0x50/0x73
>  [<c010748f>]syscall_call+0x7/0xb
> 

ooh, oww, ouch.   Look at move_one_page():

        src = get_one_pte_map_nested(mm, old_addr);
        if (src) {
                dst = alloc_one_pte_map(mm, new_addr);
                error = copy_one_pte(mm, src, dst);


get_one_pte_map_nested() does a kmap_atomic(), and then we go and
call alloc_one_pte_map->pte_alloc_map->pte_alloc_one->alloc_pages()
inside that kmap_atomic().

I guess that has been there since day one.

A simple fix would be to drop the atomic kmap of the source pte
and take it again after the alloc_one_pte_map() call.

Can you think of a more efficient way?
--
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: mremap() pte allocation atomicity error

[William Lee Irwin III]

On Fri, Sep 27, 2002 at 10:54:54PM -0700, Andrew Morton wrote:
> A simple fix would be to drop the atomic kmap of the source pte
> and take it again after the alloc_one_pte_map() call.
> Can you think of a more efficient way?

Not one that isn't highly invasive, no. This is what I had in mind
for the easy fix.


Cheers,
Bill
--
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: mremap() pte allocation atomicity error

[Andrew Morton]

William Lee Irwin III wrote:
> 
> On Fri, Sep 27, 2002 at 10:54:54PM -0700, Andrew Morton wrote:
> > A simple fix would be to drop the atomic kmap of the source pte
> > and take it again after the alloc_one_pte_map() call.
> > Can you think of a more efficient way?
> 
> Not one that isn't highly invasive, no. This is what I had in mind
> for the easy fix.
> 

OK.   kmap_atomics are pretty darn quick, but it might be better
to take a peek to see if the pgd and pmd are present, and only
drop the kmap if not.

Care to eyeball this?  I haven't tested it yet.

 mm/mremap.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+)

--- 2.5.39/mm/mremap.c~move_one_page_fix	Fri Sep 27 22:59:04 2002
+++ 2.5.39-akpm/mm/mremap.c	Fri Sep 27 23:05:16 2002
@@ -53,6 +53,20 @@ end:
 	return pte;
 }
 
+static inline int page_table_present(struct mm_struct *mm, unsigned long addr)
+{
+	pgd_t *pgd;
+	pmd_t *pmd;
+
+	pgd = pgd_offset(mm, addr);
+	if (pgd_none(*pgd))
+		return 0;
+	pmd = pmd_offset(pgd, addr);
+	if (pmd == NULL)
+		return 0;
+	return 1;
+}
+
 static inline pte_t *alloc_one_pte_map(struct mm_struct *mm, unsigned long addr)
 {
 	pmd_t * pmd;
@@ -98,7 +112,18 @@ static int move_one_page(struct vm_area_
 	spin_lock(&mm->page_table_lock);
 	src = get_one_pte_map_nested(mm, old_addr);
 	if (src) {
+		/*
+		 * Look to see whether alloc_one_pte_map needs to perform a
+		 * memory allocation.  If it does then we need to drop the
+		 * atomic kmap
+		 */
+		if (!page_table_present(mm, new_addr)) {
+			pte_unmap_nested(src);
+			src = NULL;
+		}
 		dst = alloc_one_pte_map(mm, new_addr);
+		if (src == NULL)
+			src = get_one_pte_map_nested(mm, old_addr);
 		error = copy_one_pte(mm, src, dst);
 		pte_unmap_nested(src);
 		pte_unmap(dst);

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