LU-15803 sec: correctly handle page lock in ll_io_zero_page

In ll_io_zero_page(), we need to make sure we have locked the page,
and it is up-to-date, before zeroing. So modify ll_io_read_page()
behavior to not disown the clpage for our use case. It avoids being
exposed to concurrent modifications.

Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I58e4cf80374a798c9c4302364cf2fb39da9033bb
Reviewed-on: https://review.whamcloud.com/47170
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Patrick Farrell <pfarrell@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 77aa3a7..3d200d7 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -1932,7 +1932,6 @@ int ll_io_zero_page(struct inode *inode, pgoff_t index, pgoff_t offset,
        struct cl_2queue *queue = NULL;
        struct cl_sync_io *anchor = NULL;
        bool holdinglock = false;
-       bool lockedbymyself = true;
        int rc;
 
        ENTRY;
@@ -1987,27 +1986,31 @@ int ll_io_zero_page(struct inode *inode, pgoff_t index, pgoff_t offset,
        if (!PageUptodate(vmpage) && !PageDirty(vmpage) &&
            !PageWriteback(vmpage)) {
                /* read page */
-               /* set PagePrivate2 to detect special case of empty page
-                * in osc_brw_fini_request()
+               /* Set PagePrivate2 to detect special case of empty page
+                * in osc_brw_fini_request().
+                * It is also used to tell ll_io_read_page() that we do not
+                * want the vmpage to be unlocked.
                 */
                SetPagePrivate2(vmpage);
                rc = ll_io_read_page(env, io, clpage, NULL);
-               if (!PagePrivate2(vmpage))
+               if (!PagePrivate2(vmpage)) {
                        /* PagePrivate2 was cleared in osc_brw_fini_request()
                         * meaning we read an empty page. In this case, in order
                         * to avoid allocating unnecessary block in truncated
                         * file, we must not zero and write as below. Subsequent
                         * server-side truncate will handle things correctly.
                         */
+                       cl_page_unassume(env, io, clpage);
                        GOTO(clpfini, rc = 0);
+               }
                ClearPagePrivate2(vmpage);
                if (rc)
                        GOTO(clpfini, rc);
-               lockedbymyself = trylock_page(vmpage);
-               cl_page_assume(env, io, clpage);
        }
 
-       /* zero range in page */
+       /* Thanks to PagePrivate2 flag, ll_io_read_page() did not unlock
+        * the vmpage, so we are good to proceed and zero range in page.
+        */
        zero_user(vmpage, offset, len);
 
        if (holdinglock && clpage) {
@@ -2037,10 +2040,8 @@ clpfini:
        if (clpage)
                cl_page_put(env, clpage);
 pagefini:
-       if (lockedbymyself) {
-               unlock_page(vmpage);
-               put_page(vmpage);
-       }
+       unlock_page(vmpage);
+       put_page(vmpage);
 rellock:
        if (holdinglock)
                cl_lock_release(env, lock);

diff --git a/lustre/llite/rw.c b/lustre/llite/rw.c
index 3ebfab0..b6bc138 100644 (file)
--- a/lustre/llite/rw.c
+++ b/lustre/llite/rw.c
@@ -1633,6 +1633,7 @@ int ll_io_read_page(const struct lu_env *env, struct cl_io *io,
        pgoff_t ra_start_index = 0;
        pgoff_t io_start_index;
        pgoff_t io_end_index;
+       bool unlockpage = true;
        ENTRY;
 
        if (file) {
@@ -1640,6 +1641,12 @@ int ll_io_read_page(const struct lu_env *env, struct cl_io *io,
                ras = &fd->fd_ras;
        }
 
+       /* PagePrivate2 is set in ll_io_zero_page() to tell us the vmpage
+        * must not be unlocked after processing.
+        */
+       if (page->cp_vmpage && PagePrivate2(page->cp_vmpage))
+               unlockpage = false;
+
        vpg = cl2vvp_page(cl_object_page_slice(page->cp_obj, page));
        uptodate = vpg->vpg_defer_uptodate;
 
@@ -1718,7 +1725,8 @@ int ll_io_read_page(const struct lu_env *env, struct cl_io *io,
                         * route is implemented */
                        cl_page_discard(env, io, page);
                }
-               cl_page_disown(env, io, page);
+               if (unlockpage)
+                       cl_page_disown(env, io, page);
        }
 
        /* TODO: discard all pages until page reinit route is implemented */