Comment 5 for bug 951043

Revision history for this message
Xerxes RĂ„nby (xranby) wrote :

I think there is an inconsistency in this patch that can cause a deadlock:

Consider that the code first takes the &mm->mmap_sem lock during if (!down_read_trylock(&mm->mmap_sem)) {

@@ -305,6 +297,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
  if (!down_read_trylock(&mm->mmap_sem)) {
   if (!user_mode(regs) && !search_exception_tables(regs->ARM_pc))
    goto no_context;
+retry:
   down_read(&mm->mmap_sem);
  } else {
   /*
@@ -320,14 +313,41 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 #endif
  }

and then later manage to take the goto retry;
this will will cause a deadlock when trying to take the &mm->mmap_sem twice.

+ if (flags & FAULT_FLAG_ALLOW_RETRY) {
+ if (fault & VM_FAULT_MAJOR) {
+ tsk->maj_flt++;
+ perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MAJ, 1,
+ regs, addr);
+ } else {
+ tsk->min_flt++;
+ perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, 1,
+ regs, addr);
+ }
+ if (fault & VM_FAULT_RETRY) {
+ /* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
+ * of starvation. */
+ flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ goto retry;
+ }
+ }
+
+ up_read(&mm->mmap_sem);