From: Wang Shilong <wshilong@ddn.com>
Date: Thu, 25 Jul 2019 05:55:12 +0000 (+0800)
Subject: LU-12587 llite: don't check vmpage refcount in ll_releasepage()
X-Git-Tag: 2.12.57~59
X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=e59f0c9a245f91ad929fd31dcbcbee88bf7a11df

LU-12587 llite: don't check vmpage refcount in ll_releasepage()

We could not use vmpage refcount to check whether page could be
released because it break invalidate_complete_page2():

See comments:
/*
 * This is like invalidate_complete_page(), except it ignores the page's
 * refcount.  We do this because invalidate_inode_pages2() needs stronger
 * invalidation guarantees, and cannot afford to leave pages behind because
 * shrink_page_list() has a temp ref on them, or because they're transiently
 * sitting in the lru_cache_add() pagevecs.
 */

So checking refcount > 3 might be wrong here, one common
case is page might be transiently in lru_cache_add().

Since we have checked whether vmpage is used by cl_page later in the
function, and vmpage will be locked before called, it should be safe
to remove vmpage refcount check.

One of problem currently is following DIO will mostly
fall back to Buffer IO:

 $ dd if=/dev/zero of=data bs=1M count=1
 $ dd if=/dev/zero of=data bs=1M count=1 oflag=direct conv=notrunc

Which is because DIO will firstly try to writeback and invalidate
clean page which fail because vmpage refcount could be 4 here.

Function calls come from linux-3.10.0-957.1.3.el7.x86_64:

|->generic_file_direct_write()
  |->filemap_write_and_wait_range()
    |->invalidate_inode_pages2_range()
          |->invalidate_complete_page2() If a page can not be invalidated,
					 return 0 to fall back to buffered write.
                |->try_to_release_page()
		  |->ll_releasepage()
			return 0 becuase of vmpage count is 4 > 3
   |->generic_file_buffered_write

Change-Id: I14ed0877a789d68dee2de80574688a0e699556a3
Signed-off-by: Wang Shilong <wshilong@ddn.com>
Reviewed-on: https://review.whamcloud.com/35610
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Patrick Farrell <pfarrell@whamcloud.com>
Reviewed-by: Li Dongyang <dongyangli@ddn.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
---

diff --git a/lustre/llite/rw26.c b/lustre/llite/rw26.c
index c5a4d4d..6f2bb6c 100644
--- a/lustre/llite/rw26.c
+++ b/lustre/llite/rw26.c
@@ -136,10 +136,6 @@ static int ll_releasepage(struct page *vmpage, RELEASEPAGE_ARG_TYPE gfp_mask)
 	if (obj == NULL)
 		return 1;
 
-	/* 1 for caller, 1 for cl_page and 1 for page cache */
-	if (page_count(vmpage) > 3)
-		return 0;
-
 	page = cl_vmpage_page(vmpage, obj);
 	if (page == NULL)
 		return 1;
diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 59f1210..ba2daa7 100644
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -18356,6 +18356,20 @@ test_276() {
 }
 run_test 276 "Race between mount and obd_statfs"
 
+test_277() {
+	$LCTL set_param ldlm.namespaces.*.lru_size=0
+	dd if=/dev/zero of=$DIR/$tfile bs=1M count=1
+	local cached_mb=$($LCTL get_param llite.*.max_cached_mb |
+			grep ^used_mb | awk '{print $2}')
+	[ $cached_mb -eq 1 ] || error "expected mb 1 got $cached_mb"
+	dd if=/dev/zero of=$DIR/$tfile bs=1M count=1 \
+		oflag=direct conv=notrunc
+	cached_mb=$($LCTL get_param llite.*.max_cached_mb |
+			grep ^used_mb | awk '{print $2}')
+	[ $cached_mb -eq 0 ] || error "expected mb 0 got $cached_mb"
+}
+run_test 277 "Direct IO shall drop page cache"
+
 cleanup_test_300() {
 	trap 0
 	umask $SAVE_UMASK