LU-16160 revert: "llite: clear stale page's uptodate bit"

This reverts commit 5b911e03261c3de6b0c2934c86dd191f01af4f2f
which caused a bug in cl_page_own() race with ll_releasepage()
and cl_pagevec_put() assertion failure.

Signed-off-by: Bobi Jam <bobijam@whamcloud.com>
Change-Id: Icdb8c60f4d992c9976670e1b06c5bab5ef3a3954
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49541
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Qian Yingjin <qian@ddn.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/include/cl_object.h b/lustre/include/cl_object.h
index 3daeefa..5449cef 100644 (file)
--- a/lustre/include/cl_object.h
+++ b/lustre/include/cl_object.h
@@ -767,15 +767,7 @@ struct cl_page {
        enum cl_page_type       cp_type:CP_TYPE_BITS;
        unsigned                cp_defer_uptodate:1,
                                cp_ra_updated:1,
-                               cp_ra_used:1,
-                               /* fault page read grab extra referece */
-                               cp_fault_ref:1,
-                               /**
-                                * if fault page got delete before returned to
-                                * filemap_fault(), defer the vmpage detach/put
-                                * until filemap_fault() has been handled.
-                                */
-                               cp_defer_detach:1;
+                               cp_ra_used:1;
        /* which slab kmem index this memory allocated from */
        short int               cp_kmem_index;
 
@@ -2397,11 +2389,6 @@ static inline int cl_io_is_mkwrite(const struct cl_io *io)
        return io->ci_type == CIT_FAULT && io->u.ci_fault.ft_mkwrite;
 }
 
-static inline int cl_io_is_pagefault(const struct cl_io *io)
-{
-       return io->ci_type == CIT_FAULT && !io->u.ci_fault.ft_mkwrite;
-}
-
 /**
  * True, iff \a io is a truncate(2).
  */

diff --git a/lustre/llite/rw.c b/lustre/llite/rw.c
index 0f76a88..66496e6 100644 (file)
--- a/lustre/llite/rw.c
+++ b/lustre/llite/rw.c
@@ -1943,15 +1943,7 @@ int ll_readpage(struct file *file, struct page *vmpage)
                        unlock_page(vmpage);
                        result = 0;
                }
-               if (cl_io_is_pagefault(io) && result == 0) {
-                       /**
-                        * page fault, retain the cl_page reference until
-                        * vvp_io_kernel_fault() release it.
-                        */
-                       page->cp_fault_ref = 1;
-               } else {
-                       cl_page_put(env, page);
-               }
+               cl_page_put(env, page);
        } else {
                unlock_page(vmpage);
                result = PTR_ERR(page);

diff --git a/lustre/llite/vvp_io.c b/lustre/llite/vvp_io.c
index 3c500a5..f1cb18d 100644 (file)
--- a/lustre/llite/vvp_io.c
+++ b/lustre/llite/vvp_io.c
@@ -1391,37 +1391,10 @@ static void vvp_io_rw_end(const struct lu_env *env,
        trunc_sem_up_read(&lli->lli_trunc_sem);
 }
 
-static void detach_and_deref_page(struct cl_page *clp, struct page *vmpage)
-{
-       if (!clp->cp_defer_detach)
-               return;
-
-       /**
-        * cl_page_delete0() took a vmpage reference, but not unlink the vmpage
-        * from its cl_page.
-        */
-       clp->cp_defer_detach = 0;
-       ClearPagePrivate(vmpage);
-       vmpage->private = 0;
-
-       put_page(vmpage);
-       refcount_dec(&clp->cp_ref);
-}
-
-static int vvp_io_kernel_fault(const struct lu_env *env,
-                              struct vvp_fault_io *cfio)
+static int vvp_io_kernel_fault(struct vvp_fault_io *cfio)
 {
        struct vm_fault *vmf = cfio->ft_vmf;
-       struct file *vmff = cfio->ft_vma->vm_file;
-       struct address_space *mapping = vmff->f_mapping;
-       struct inode *inode = mapping->host;
-       struct page *vmpage = NULL;
-       struct cl_page *clp = NULL;
-       int rc = 0;
-       ENTRY;
 
-       ll_inode_size_lock(inode);
-retry:
        cfio->ft_flags = ll_filemap_fault(cfio->ft_vma, vmf);
        cfio->ft_flags_valid = 1;
 
@@ -1436,100 +1409,25 @@ retry:
 
                cfio->ft_vmpage = vmf->page;
 
-               /**
-                * ll_filemap_fault()->ll_readpage() could get an extra cl_page
-                * reference. So we have to get the cl_page's to check its
-                * cp_fault_ref and drop the reference later.
-                */
-               clp = cl_vmpage_page(vmf->page, NULL);
-
-               GOTO(unlock, rc = 0);
-       }
-
-       /* filemap_fault() fails, vmpage is not locked */
-       if (clp == NULL) {
-               vmpage = find_get_page(mapping, vmf->pgoff);
-               if (vmpage) {
-                       lock_page(vmpage);
-                       clp = cl_vmpage_page(vmpage, NULL);
-                       unlock_page(vmpage);
-               }
+               return 0;
        }
 
        if (cfio->ft_flags & VM_FAULT_SIGBUS) {
-               pgoff_t max_idx;
-
-               /**
-                * ll_filemap_fault()->ll_readpage() could fill vmpage
-                * correctly, and unlock the vmpage, while memory pressure or
-                * truncate could detach cl_page from vmpage, and kernel
-                * filemap_fault() will wait_on_page_locked(vmpage) and find
-                * out that the vmpage has been cleared its uptodate bit,
-                * so it returns VM_FAULT_SIGBUS.
-                *
-                * In this case, we'd retry the filemap_fault()->ll_readpage()
-                * to rebuild the cl_page and fill vmpage with uptodated data.
-                */
-               if (likely(vmpage)) {
-                       bool need_retry = false;
-
-                       if (clp) {
-                               if (clp->cp_defer_detach) {
-                                       detach_and_deref_page(clp, vmpage);
-                                       /**
-                                        * check i_size to make sure it's not
-                                        * over EOF, we don't want to call
-                                        * filemap_fault() repeatedly since it
-                                        * returns VM_FAULT_SIGBUS without even
-                                        * trying if vmf->pgoff is over EOF.
-                                        */
-                                       max_idx = DIV_ROUND_UP(
-                                               i_size_read(inode), PAGE_SIZE);
-                                       if (vmf->pgoff < max_idx)
-                                               need_retry = true;
-                               }
-                               if (clp->cp_fault_ref) {
-                                       clp->cp_fault_ref = 0;
-                                       /* ref not released in ll_readpage() */
-                                       cl_page_put(env, clp);
-                               }
-                               if (need_retry)
-                                       goto retry;
-                       }
-               }
-
                CDEBUG(D_PAGE, "got addr %p - SIGBUS\n", get_vmf_address(vmf));
-               GOTO(unlock, rc = -EFAULT);
+               return -EFAULT;
        }
 
        if (cfio->ft_flags & VM_FAULT_OOM) {
                CDEBUG(D_PAGE, "got addr %p - OOM\n", get_vmf_address(vmf));
-               GOTO(unlock, rc = -ENOMEM);
+               return -ENOMEM;
        }
 
        if (cfio->ft_flags & VM_FAULT_RETRY)
-               GOTO(unlock, rc = -EAGAIN);
+               return -EAGAIN;
 
        CERROR("unknown error in page fault %d\n", cfio->ft_flags);
-       rc = -EINVAL;
-unlock:
-       ll_inode_size_unlock(inode);
-       if (clp) {
-               if (clp->cp_defer_detach && vmpage)
-                       detach_and_deref_page(clp, vmpage);
-
-               /* additional cl_page ref has been taken in ll_readpage() */
-               if (clp->cp_fault_ref) {
-                       clp->cp_fault_ref = 0;
-                       /* ref not released in ll_readpage() */
-                       cl_page_put(env, clp);
-               }
-               /* ref taken in this function */
-               cl_page_put(env, clp);
-       }
-       if (vmpage)
-               put_page(vmpage);
-       return rc;
+
+       return -EINVAL;
 }
 
 static void mkwrite_commit_callback(const struct lu_env *env, struct cl_io *io,
@@ -1570,7 +1468,7 @@ static int vvp_io_fault_start(const struct lu_env *env,
                LASSERT(cfio->ft_vmpage != NULL);
                lock_page(cfio->ft_vmpage);
        } else {
-               result = vvp_io_kernel_fault(env, cfio);
+               result = vvp_io_kernel_fault(cfio);
                if (result != 0)
                        RETURN(result);
        }

diff --git a/lustre/obdclass/cl_page.c b/lustre/obdclass/cl_page.c
index bc384be..92ba1d6 100644 (file)
--- a/lustre/obdclass/cl_page.c
+++ b/lustre/obdclass/cl_page.c
@@ -827,35 +827,17 @@ static void __cl_page_delete(const struct lu_env *env, struct cl_page *cp)
                LASSERT(PageLocked(vmpage));
                LASSERT((struct cl_page *)vmpage->private == cp);
 
-               /**
-                * clear vmpage uptodate bit, since ll_read_ahead_pages()->
-                * ll_read_ahead_page() could pick up this stale vmpage and
-                * take it as uptodated.
-                */
-               ClearPageUptodate(vmpage);
-               /**
-                * vvp_io_kernel_fault()->ll_readpage() set cp_fault_ref
-                * and need it to check cl_page to retry the page fault read.
+               /* Drop the reference count held in vvp_page_init */
+               refcount_dec(&cp->cp_ref);
+
+               ClearPagePrivate(vmpage);
+               vmpage->private = 0;
+
+               /*
+                * The reference from vmpage to cl_page is removed,
+                * but the reference back is still here. It is removed
+                * later in cl_page_free().
                 */
-               if (cp->cp_fault_ref) {
-                       cp->cp_defer_detach = 1;
-                       /**
-                        * get a vmpage reference, so that filemap_fault()
-                        * won't free it from pagecache.
-                        */
-                       get_page(vmpage);
-               } else {
-                       /* Drop the reference count held in vvp_page_init */
-                       refcount_dec(&cp->cp_ref);
-                       ClearPagePrivate(vmpage);
-                       vmpage->private = 0;
-
-                       /*
-                        * The reference from vmpage to cl_page is removed,
-                        * but the reference back is still here. It is removed
-                        * later in cl_page_free().
-                        */
-               }
        }
 
        EXIT;

diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh
index ce4968e..8019af2 100755 (executable)
--- a/lustre/tests/sanityn.sh
+++ b/lustre/tests/sanityn.sh
@@ -18,8 +18,8 @@ init_test_env $@
 init_logging
 
 ALWAYS_EXCEPT="$SANITYN_EXCEPT "
-# bug number for skipped test:  LU-7105
-ALWAYS_EXCEPT+="                28 "
+# bug number for skipped test:  LU-7105 LU-14541 LU-16160
+ALWAYS_EXCEPT+="                28      16f      16g "
 # UPDATE THE COMMENT ABOVE WITH BUG NUMBERS WHEN CHANGING ALWAYS_EXCEPT!
 
 if [ $mds1_FSTYPE = "zfs" ]; then