Whamcloud - gitweb
LU-122 Revert the patch on bug 21122 and come up with a new fix
authorJinshan Xiong <jay@whamcloud.com>
Fri, 1 Apr 2011 04:24:43 +0000 (21:24 -0700)
committerOleg Drokin <green@whamcloud.com>
Mon, 25 Apr 2011 17:16:19 +0000 (10:16 -0700)
The patch on bug 21122 will cause deadlock. The root cause of
the issue is that a page was truncated even with cl_lock held.

Signed-off-by: Jinshan Xiong <jay@whamcloud.com>
Change-Id: If04a5632e64a019803465533d3ec7dba49e42168
Reviewed-on: http://review.whamcloud.com/316
Tested-by: Hudson
Reviewed-by: wangdi <di.wang@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/llite/llite_mmap.c
lustre/llite/vvp_io.c
lustre/obdclass/cl_lock.c

index c955661..9611d6b 100644 (file)
@@ -69,6 +69,9 @@
                vma->vm_file->f_dentry->d_inode->i_ino,                       \
                vma->vm_file->f_dentry->d_iname, ## arg);                     \
 
+struct page *ll_nopage(struct vm_area_struct *vma, unsigned long address,
+                       int *type);
+
 static struct vm_operations_struct ll_file_vm_ops;
 
 void policy_from_vma(ldlm_policy_data_t *policy,
@@ -203,8 +206,8 @@ struct page *ll_nopage(struct vm_area_struct *vma, unsigned long address,
         struct lu_env           *env;
         struct cl_env_nest      nest;
         struct cl_io            *io;
-        struct page             *page;
-        struct vvp_io           *vio;
+        struct page             *page  = NOPAGE_SIGBUS;
+        struct vvp_io           *vio = NULL;
         unsigned long           ra_flags;
         pgoff_t                 pg_offset;
         int                     result;
@@ -220,26 +223,21 @@ struct page *ll_nopage(struct vm_area_struct *vma, unsigned long address,
                 goto out_err;
 
         vio = vvp_env_io(env);
+
         vio->u.fault.ft_vma            = vma;
-        vio->u.fault.ft_vmpage         = NULL;
         vio->u.fault.nopage.ft_address = address;
         vio->u.fault.nopage.ft_type    = type;
 
         result = cl_io_loop(env, io);
 
-        page = vio->u.fault.ft_vmpage;
-        if (page != NULL) {
-                LASSERT(PageLocked(page));
-                unlock_page(page);
-
-                if (result != 0)
-                        page_cache_release(page);
-        }
-
-        LASSERT(ergo(result == 0, io->u.ci_fault.ft_page != NULL));
 out_err:
-        if (result != 0)
-                page = result == -ENOMEM ? NOPAGE_OOM : NOPAGE_SIGBUS;
+        if (result == 0) {
+                LASSERT(io->u.ci_fault.ft_page != NULL);
+                page = vio->u.fault.ft_vmpage;
+        } else {
+                if (result == -ENOMEM)
+                        page = NOPAGE_OOM;
+        }
 
         vma->vm_flags &= ~VM_RAND_READ;
         vma->vm_flags |= ra_flags;
@@ -261,11 +259,11 @@ out_err:
  * \retval VM_FAULT_ERROR on general error
  * \retval NOPAGE_OOM not have memory for allocate new page
  */
-int ll_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+int ll_fault0(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
         struct lu_env           *env;
         struct cl_io            *io;
-        struct vvp_io           *vio;
+        struct vvp_io           *vio = NULL;
         unsigned long            ra_flags;
         struct cl_env_nest       nest;
         int                      result;
@@ -286,17 +284,8 @@ int ll_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
         vio->u.fault.fault.ft_vmf = vmf;
 
         result = cl_io_loop(env, io);
-        if (unlikely(result != 0 && vio->u.fault.ft_vmpage != NULL)) {
-                struct page *vmpage = vio->u.fault.ft_vmpage;
-
-                LASSERT((vio->u.fault.fault.ft_flags & VM_FAULT_LOCKED) &&
-                        PageLocked(vmpage));
-                unlock_page(vmpage);
-                page_cache_release(vmpage);
-                vmf->page = NULL;
-        }
-
         fault_ret = vio->u.fault.fault.ft_flags;
+
 out_err:
         if (result != 0)
                 fault_ret |= VM_FAULT_ERROR;
@@ -309,6 +298,39 @@ out_err:
         RETURN(fault_ret);
 }
 
+int ll_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+        int count = 0;
+        bool printed = false;
+        int result;
+
+restart:
+        result = ll_fault0(vma, vmf);
+        LASSERT(!(result & VM_FAULT_LOCKED));
+        if (result == 0) {
+                struct page *vmpage = vmf->page;
+
+                /* check if this page has been truncated */
+                lock_page(vmpage);
+                if (unlikely(vmpage->mapping == NULL)) { /* unlucky */
+                        unlock_page(vmpage);
+                        page_cache_release(vmpage);
+                        vmf->page = NULL;
+
+                        if (!printed && ++count > 16) {
+                                CWARN("the page is under heavy contention,"
+                                      "maybe your app(%s) needs revising :-)\n",
+                                      current->comm);
+                                printed = true;
+                        }
+
+                        goto restart;
+                }
+
+                result |= VM_FAULT_LOCKED;
+        }
+        return result;
+}
 #endif
 
 /**
index 502ec4e..4d217c9 100644 (file)
@@ -634,14 +634,6 @@ static int vvp_io_kernel_fault(struct vvp_fault_io *cfio)
         LL_CDEBUG_PAGE(D_PAGE, vmpage, "got addr %lu type %lx\n",
                        cfio->nopage.ft_address, (long)cfio->nopage.ft_type);
 
-        lock_page(vmpage);
-        if (vmpage->mapping == NULL) {
-                CERROR("vmpage %lu@%p was truncated!\n", vmpage->index, vmpage);
-                unlock_page(vmpage);
-                page_cache_release(vmpage);
-                return -EFAULT;
-        }
-
         cfio->ft_vmpage = vmpage;
 
         return 0;
@@ -655,7 +647,12 @@ static int vvp_io_kernel_fault(struct vvp_fault_io *cfio)
                 LL_CDEBUG_PAGE(D_PAGE, cfio->fault.ft_vmf->page,
                                "got addr %p type NOPAGE\n",
                                cfio->fault.ft_vmf->virtual_address);
-                LASSERT(cfio->fault.ft_flags & VM_FAULT_LOCKED);
+                /*XXX workaround to bug in CLIO - he deadlocked with
+                 lock cancel if page locked  */
+                if (likely(cfio->fault.ft_flags & VM_FAULT_LOCKED)) {
+                        unlock_page(cfio->fault.ft_vmf->page);
+                        cfio->fault.ft_flags &= ~VM_FAULT_LOCKED;
+                }
 
                 cfio->ft_vmpage = cfio->fault.ft_vmf->page;
                 return 0;
@@ -708,15 +705,25 @@ static int vvp_io_fault_start(const struct lu_env *env,
         if (result != 0)
                 return result;
 
-        /* must return locked page */
+        /* must return unlocked page */
         kernel_result = vvp_io_kernel_fault(cfio);
         if (kernel_result != 0)
                 return kernel_result;
 
-        page = cl_page_find(env, obj, fio->ft_index, cfio->ft_vmpage,
-                            CPT_CACHEABLE);
-        if (IS_ERR(page))
+        /* Temporarily lock vmpage to keep cl_page_find() happy. */
+        lock_page(cfio->ft_vmpage);
+        /* Though we have already held a cl_lock upon this page, but
+         * it still can be truncated locally. */
+        page = ERR_PTR(-EFAULT);
+        if (likely(cfio->ft_vmpage->mapping != NULL))
+                page = cl_page_find(env, obj, fio->ft_index, cfio->ft_vmpage,
+                                    CPT_CACHEABLE);
+        unlock_page(cfio->ft_vmpage);
+        if (IS_ERR(page)) {
+                page_cache_release(cfio->ft_vmpage);
+                cfio->ft_vmpage = NULL;
                 return PTR_ERR(page);
+        }
 
         size = i_size_read(inode);
         last = cl_index(obj, size - 1);
index 0e24306..de487f6 100644 (file)
@@ -1915,7 +1915,7 @@ void cl_lock_page_list_fixup(const struct lu_env *env,
                         page->cp_index < temp->cp_index));
 
                 found = cl_lock_at_page(env, lock->cll_descr.cld_obj,
-                                        page, lock, 0, 0);
+                                        page, lock, 1, 0);
                 if (found == NULL)
                         continue;