From: Sebastien Buisson <sbuisson@ddn.com>
Date: Thu, 28 Apr 2022 13:34:57 +0000 (+0200)
Subject: LU-15803 sec: correctly handle page lock in ll_io_zero_page
X-Git-Tag: 2.15.0-RC4~3
X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=edcd05e5ac035dd1daf263105df33d7cfc6dbf6e;p=fs%2Flustre-release.git

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
--- 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
--- 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 */