Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Gao Xiang <hsiangkao@aol.com>
To:     linux-erofs@lists.ozlabs.org
Cc:     Chao Yu <yuchao0@huawei.com>, Chao Yu <chao@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Gao Xiang <hsiangkao@redhat.com>
Subject: [PATCH 4/4] erofs: complete a missing case for inplace I/O
Date:   Thu, 22 Oct 2020 22:57:24 +0800
Message-Id: <20201022145724.27284-4-hsiangkao@aol.com>
In-Reply-To: <20201022145724.27284-1-hsiangkao@aol.com>
References: <20201022145724.27284-1-hsiangkao@aol.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Length: 2466
Precedence: bulk

From: Gao Xiang <hsiangkao@redhat.com>

Add a missing case which could cause unnecessary page allocation
but not directly use inplace I/O instead, which increases runtime
extra memory footprint.

The detail is, considering a file-backed page, the right half of
the page is chosen to be cached (e.g. the end page) and some of
its data doesn't exist in managed cache, so the pcluster will be
kept in the submission chain. (In other words, it cannot be
decompressed in advance, for example, due to the bypass chain).

Currently, the pcluster for this case is downgraded as NOINPLACE,
and stop the left half of the page from doing inplace I/O (so an
extra page allocation is needed then.)

Let's avoid such unnecessary downgrade instead.

Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 fs/erofs/zdata.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index edd7325570e1..ef12275f7fcc 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -160,7 +160,8 @@ static void preload_compressed_pages(struct z_erofs_collector *clt,
 	const unsigned int clusterpages = BIT(pcl->clusterbits);
 	struct page **pages = clt->compressedpages;
 	pgoff_t index = pcl->obj.index + (pages - pcl->compressed_pages);
-	bool standalone = true;
+	bool io_cant_bypass = false;
+	bool updated = false;
 
 	if (clt->mode < COLLECT_PRIMARY_FOLLOWED)
 		return;
@@ -180,20 +181,31 @@ static void preload_compressed_pages(struct z_erofs_collector *clt,
 		} else if (type == DELAYEDALLOC) {
 			t = tagptr_init(compressed_page_t, PAGE_UNALLOCATED);
 		} else {	/* DONTALLOC */
-			if (standalone)
+			/* update to first inplace I/O page */
+			if (!updated) {
 				clt->compressedpages = pages;
-			standalone = false;
+				updated = true;
+			}
+			io_cant_bypass = true;
 			continue;
 		}
 
-		if (!cmpxchg_relaxed(pages, NULL, tagptr_cast_ptr(t)))
+		if (!cmpxchg_relaxed(pages, NULL, tagptr_cast_ptr(t))) {
+			if (type == DELAYEDALLOC)
+				io_cant_bypass = true;
 			continue;
+		}
 
 		if (page)
 			put_page(page);
 	}
 
-	if (standalone)		/* downgrade to PRIMARY_FOLLOWED_NOINPLACE */
+	/*
+	 * for COLLECT_PRIMARY_FOLLOWED pcluster, if all managed pages have
+	 * been found (which means it can be handled without submittng I/O)
+	 * it's unsafe to do inplace I/O. let's downgrade to NOINPLACE instead.
+	 */
+	if (!io_cant_bypass)
 		clt->mode = COLLECT_PRIMARY_FOLLOWED_NOINPLACE;
 }
 
-- 
2.24.0