Messages in this thread Patch in this message | | | Date | Thu, 07 Aug 1997 13:45:23 -0400 | From | Bill Hawes <> | Subject | snapshot of fork/dup_mmap patch |
| |
I've attached a patch of the current dup_mmap patch. I've added a flag to the task structure to indicate that the task is holding the tsk->mm->mmap_sem semaphore, and test this in do_page_fault to see whether it's necessary to acquire the semaphore. This should avoid problems with page faults while holding mmap_sem.
The patch works OK, but I haven't seen the printk message from do_page_fault. Perhaps Philip can trigger it on his busy machine ...
The tsk->sem_locked flag should be useful to kswapd in deciding whether to get the semaphore.
Next (and hopefully final) step: preallocate pages to avoid allocating in pte_alloc. It doesn't look too hard to go through the vma structs and add up how many pages would be needed. Passing them in through copy_page_range would be ugly, so I might a link them into a list and add it to the task structure. Also ugly ... any suggestions?
Regards, Bill--- linux-2.0.30/kernel/fork.c.old Sun Apr 27 15:54:44 1997 +++ linux-2.0.30/kernel/fork.c Wed Aug 6 08:31:40 1997 @@ -77,22 +77,82 @@ return last_pid; } +/* + * Acquire the current task's mmap semaphore, and inform the page-fault + * handler that we already have the lock. This allows us to generate + * page-faults while working on our mmap structures. + */ +static inline void down_current(void) +{ + down(¤t->mm->mmap_sem); + current->sem_locked = 1; +} + +static inline void up_current(void) +{ + current->sem_locked = 0; + up(¤t->mm->mmap_sem); +} + +/* + * There are some serious race conditions here if the current process is + * a cloned process, as its (shared) mmap may change while we're trying + * to copy it. Holding the semaphore has risks too, as some of the copy + * operations may generate page faults. + */ static inline int dup_mmap(struct mm_struct * mm) { struct vm_area_struct * mpnt, **p, *tmp; + struct vm_area_struct * free = NULL; + int need, have=0; + int retval = -ENOMEM; + + /* + * Acquire the mmap semaphore, and then count how many vm_area_structs + * we need to do the copy. If we don't have enough yet, release the + * semaphore and allocate more. + */ +count: + down_current(); + need = 0; + for (mpnt = current->mm->mmap; mpnt ; mpnt = mpnt->vm_next) + need++; + if (need <= have) + goto begin_copy; + up_current(); + + /* + * We don't have enough vm_area_structs ... allocate more and put + * them on our private free list. + */ + while (need > have++) { + tmp = (struct vm_area_struct *) + kmalloc(sizeof(struct vm_area_struct), GFP_KERNEL); + if (!tmp) + goto free_mem_out; + tmp->vm_next = free; + free = tmp; + } + goto count; + /* + * We now have enough vm_area_structs and are holding the mmap + * semaphore, so it's safe to start the copy operation. + */ +begin_copy: mm->mmap = NULL; p = &mm->mmap; for (mpnt = current->mm->mmap ; mpnt ; mpnt = mpnt->vm_next) { - tmp = (struct vm_area_struct *) kmalloc(sizeof(struct vm_area_struct), GFP_KERNEL); - if (!tmp) { - exit_mmap(mm); - return -ENOMEM; - } + tmp = free; + free = tmp->vm_next; *tmp = *mpnt; tmp->vm_flags &= ~VM_LOCKED; tmp->vm_mm = mm; tmp->vm_next = NULL; + + /* + * Can this be deferred until copy_page_range succeeds? + */ if (tmp->vm_inode) { tmp->vm_inode->i_count++; /* insert tmp into the share list, just after mpnt */ @@ -100,43 +160,96 @@ mpnt->vm_next_share = tmp; tmp->vm_prev_share = mpnt; } - if (copy_page_range(mm, current->mm, tmp)) { - exit_mmap(mm); - return -ENOMEM; + + /* + * If this blocks, vmtruncate() could mess with the pages. + * Check that out ... + */ + retval = copy_page_range(mm, current->mm, tmp); + + if (!retval) { + /* do we need a flag saying it was opened? */ + if (tmp->vm_ops && tmp->vm_ops->open) + tmp->vm_ops->open(tmp); } - if (tmp->vm_ops && tmp->vm_ops->open) - tmp->vm_ops->open(tmp); + + /* + * Link the vma struct into the list even if we had an error, + * so exit_mmap can clean up the mess. + */ *p = tmp; p = &tmp->vm_next; + if (retval) + goto exit_mmap_out; } + up_current(); build_mmap_avl(mm); - return 0; + retval = 0; + + /* + * It's possible that we have leftover vm_area_structs, + * so we just release everything on the free list. + */ +free_mem_out: + while ((tmp = free) != NULL) { + free = tmp->vm_next; + kfree(tmp); + } + return retval; + + /* + * If we have a partially built mmap, release it here. + */ +exit_mmap_out: + up_current(); + printk("dup_mmap: failure, cleaning up mm\n"); + exit_mmap(mm); + goto free_mem_out; } static inline int copy_mm(unsigned long clone_flags, struct task_struct * tsk) { - if (!(clone_flags & CLONE_VM)) { - struct mm_struct * mm = kmalloc(sizeof(*tsk->mm), GFP_KERNEL); - if (!mm) - return -1; - *mm = *current->mm; - mm->count = 1; - mm->def_flags = 0; - tsk->mm = mm; - tsk->min_flt = tsk->maj_flt = 0; - tsk->cmin_flt = tsk->cmaj_flt = 0; - tsk->nswap = tsk->cnswap = 0; - if (new_page_tables(tsk)) - return -1; - if (dup_mmap(mm)) { - free_page_tables(mm); - return -1; - } + struct mm_struct * mm; + int retval; + + if (clone_flags & CLONE_VM) { + SET_PAGE_DIR(tsk, current->mm->pgd); + current->mm->count++; return 0; } - SET_PAGE_DIR(tsk, current->mm->pgd); - current->mm->count++; + + retval = -ENOMEM; + mm = kmalloc(sizeof(*tsk->mm), GFP_KERNEL); + if (!mm) + goto fail_nomem; + *mm = *current->mm; + mm->count = 1; + mm->def_flags = 0; + /* + * If current is a cloned process, the semaphore may be + * in use, and we don't want a copy of a busy semaphore! + */ + mm->mmap_sem = MUTEX; + tsk->mm = mm; + tsk->min_flt = tsk->maj_flt = 0; + tsk->cmin_flt = tsk->cmaj_flt = 0; + tsk->nswap = tsk->cnswap = 0; + retval = new_page_tables(tsk); + if (retval) + goto cleanup_mm; + + retval = dup_mmap(mm); + if (retval) + goto cleanup_page_tables; return 0; + +cleanup_page_tables: + free_page_tables(mm); +cleanup_mm: + tsk->mm = NULL; + kfree(mm); +fail_nomem: + return retval; } static inline int copy_fs(unsigned long clone_flags, struct task_struct * tsk) @@ -290,9 +403,9 @@ bad_fork_cleanup_sighand: exit_sighand(p); bad_fork_cleanup_fs: - exit_fs(p); + exit_fs(p); /* blocking */ bad_fork_cleanup_files: - exit_files(p); + exit_files(p); /* blocking */ bad_fork_cleanup: if (p->exec_domain && p->exec_domain->use_count) (*p->exec_domain->use_count)--; --- linux-2.0.30/arch/i386/mm/fault.c.old Wed Sep 11 10:57:13 1996 +++ linux-2.0.30/arch/i386/mm/fault.c Wed Aug 6 09:29:54 1997 @@ -46,7 +46,13 @@ /* get the address */ __asm__("movl %%cr2,%0":"=r" (address)); - down(&mm->mmap_sem); + /* + * Don't need the mmap semaphore if the current task already has it. + */ + if (!tsk->sem_locked) + down(&mm->mmap_sem); + else + printk("do_page_fault: didn't need semaphore\n"); vma = find_vma(mm, address); if (!vma) goto bad_area; @@ -93,7 +99,8 @@ goto bad_area; } handler(tsk, vma, address, write); - up(&mm->mmap_sem); + if (!tsk->sem_locked) + up(&mm->mmap_sem); /* * Did it hit the DOS screen memory VA from vm86 mode? */ @@ -109,7 +116,8 @@ * Fix it, but check if it's kernel or user first.. */ bad_area: - up(&mm->mmap_sem); + if (!tsk->sem_locked) + up(&mm->mmap_sem); if (error_code & 4) { tsk->tss.cr2 = address; tsk->tss.error_code = error_code; --- linux-2.0.30/include/linux/sched.h.old Mon Jul 21 17:47:45 1997 +++ linux-2.0.30/include/linux/sched.h Wed Aug 6 11:56:22 1997 @@ -217,7 +217,8 @@ long utime, stime, cutime, cstime, start_time; /* mm fault and swap info: this can arguably be seen as either mm-specific or thread-specific */ unsigned long min_flt, maj_flt, nswap, cmin_flt, cmaj_flt, cnswap; - int swappable:1; +/* swp */ + int swappable:1, sem_locked:1; unsigned long swap_address; unsigned long old_maj_flt; /* old value of maj_flt */ unsigned long dec_flt; /* page fault count of the last time */ @@ -297,7 +298,7 @@ /* timer */ { NULL, NULL, 0, 0, it_real_fn }, \ /* utime */ 0,0,0,0,0, \ /* flt */ 0,0,0,0,0,0, \ -/* swp */ 0,0,0,0,0, \ +/* swp */ 0,0,0,0,0,0, \ /* rlimits */ INIT_RLIMITS, \ /* math */ 0, \ /* comm */ "swapper", \ | |