From: Xiubo Li <xiubli@redhat.com>
Date: Mon, 11 Jul 2022 12:11:21 +0800
Subject: netfs: do not unlock and put the folio twice
Git-commit: fac47b43c760ea90e64b895dba60df0327be7775
Patch-mainline: v5.19-rc7
References: jsc#SES-1880

check_write_begin() will unlock and put the folio when return
non-zero.  So we should avoid unlocking and putting it twice in
netfs layer.

Change the way ->check_write_begin() works in the following two ways:

 (1) Pass it a pointer to the folio pointer, allowing it to unlock and put
     the folio prior to doing the stuff it wants to do, provided it clears
     the folio pointer.

 (2) Change the return values such that 0 with folio pointer set means
     continue, 0 with folio pointer cleared means re-get and all error
     codes indicating an error (no special treatment for -EAGAIN).

[ bagasdotme: use Sphinx code text syntax for *foliop pointer ]

Cc: stable@vger.kernel.org
Link: https://tracker.ceph.com/issues/56423
Link: https://lore.kernel.org/r/cf169f43-8ee7-8697-25da-0204d1b4343e@redhat.com
Co-developed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Acked-by: Luis Henriques <lhenriques@suse.com>
---
 Documentation/filesystems/netfs_library.rst |    8 +++++---
 fs/afs/file.c                               |    2 +-
 fs/ceph/addr.c                              |   11 ++++++-----
 fs/netfs/buffered_read.c                    |   17 ++++++++++-------
 include/linux/netfs.h                       |    2 +-
 5 files changed, 23 insertions(+), 17 deletions(-)

--- a/Documentation/filesystems/netfs_library.rst
+++ b/Documentation/filesystems/netfs_library.rst
@@ -302,7 +302,7 @@ through which it can issue requests and
 		void (*issue_read)(struct netfs_io_subrequest *subreq);
 		bool (*is_still_valid)(struct netfs_io_request *rreq);
 		int (*check_write_begin)(struct file *file, loff_t pos, unsigned len,
-					 struct page *page, void **_fsdata);
+					 struct page **pagep, void **_fsdata);
 		void (*done)(struct netfs_io_request *rreq);
 	};
 
@@ -381,8 +381,10 @@ The operations are as follows:
    allocated/grabbed the page to be modified to allow the filesystem to flush
    conflicting state before allowing it to be modified.
 
-   It should return 0 if everything is now fine, -EAGAIN if the page should be
-   regrabbed and any other error code to abort the operation.
+   It may unlock and discard the page it was given and set the caller's page
+   pointer to NULL.  It should return 0 if everything is now fine (``*pagep``
+   left set) or the op should be retried (``*pagep`` cleared) and any other
+   error code to abort the operation.
 
  * ``done``
 
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -367,7 +367,7 @@ static int afs_begin_cache_operation(str
 }
 
 static int afs_check_write_begin(struct file *file, loff_t pos, unsigned len,
-				 struct page *page, void **_fsdata)
+				 struct page **pagep, void **_fsdata)
 {
 	struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
 
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -63,7 +63,7 @@
 	 (CONGESTION_ON_THRESH(congestion_kb) >> 2))
 
 static int ceph_netfs_check_write_begin(struct file *file, loff_t pos, unsigned int len,
-					struct page *page, void **_fsdata);
+					struct page **pagep, void **_fsdata);
 
 static inline struct ceph_snap_context *page_snap_context(struct page *page)
 {
@@ -1278,18 +1278,19 @@ ceph_find_incompatible(struct page *page
 }
 
 static int ceph_netfs_check_write_begin(struct file *file, loff_t pos, unsigned int len,
-					struct page *page, void **_fsdata)
+					struct page **pagep, void **_fsdata)
 {
 	struct inode *inode = file_inode(file);
 	struct ceph_inode_info *ci = ceph_inode(inode);
 	struct ceph_snap_context *snapc;
 
-	snapc = ceph_find_incompatible(page);
+	snapc = ceph_find_incompatible(*pagep);
 	if (snapc) {
 		int r;
 
-		unlock_page(page);
-		put_page(page);
+		unlock_page(*pagep);
+		put_page(*pagep);
+		*pagep = NULL;
 		if (IS_ERR(snapc))
 			return PTR_ERR(snapc);
 
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -322,8 +322,9 @@ zero_out:
  * conflicting writes once the page is grabbed and locked.  It is passed a
  * pointer to the fsdata cookie that gets returned to the VM to be passed to
  * write_end.  It is permitted to sleep.  It should return 0 if the request
- * should go ahead; unlock the page and return -EAGAIN to cause the page to be
- * regot; or return an error.
+ * should go ahead or it may return an error.  It may also unlock and put the
+ * page, provided it sets ``*pagep`` to NULL, in which case a return of 0
+ * will cause the page to be re-got and the process to be retried.
  *
  * The calling netfs must initialise a netfs context contiguous to the vfs
  * inode before calling this.
@@ -349,13 +350,13 @@ retry:
 
 	if (ctx->ops->check_write_begin) {
 		/* Allow the netfs (eg. ceph) to flush conflicts. */
-		ret = ctx->ops->check_write_begin(file, pos, len, page, _fsdata);
+		ret = ctx->ops->check_write_begin(file, pos, len, &page, _fsdata);
 		if (ret < 0) {
 			trace_netfs_failure(NULL, NULL, ret, netfs_fail_check_write_begin);
-			if (ret == -EAGAIN)
-				goto retry;
 			goto error;
 		}
+		if (!page)
+			goto retry;
 	}
 
 	if (PageUptodate(page))
@@ -417,8 +418,10 @@ have_page_no_wait:
 error_put:
 	netfs_put_request(rreq, false, netfs_rreq_trace_put_failed);
 error:
-	unlock_page(page);
-	put_page(page);
+	if (page) {
+		unlock_page(page);
+		put_page(page);
+	}
 	_leave(" = %d", ret);
 	return ret;
 }
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -192,7 +192,7 @@ struct netfs_request_ops {
 	void (*issue_read)(struct netfs_io_subrequest *subreq);
 	bool (*is_still_valid)(struct netfs_io_request *rreq);
 	int (*check_write_begin)(struct file *file, loff_t pos, unsigned len,
-				 struct page *page, void **_fsdata);
+				 struct page **pagep, void **_fsdata);
 	void (*done)(struct netfs_io_request *rreq);
 };