All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruenba@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH v5 5/9] iomap/gfs2: Get page in page_prepare handler
Date: Sat, 31 Dec 2022 16:09:15 +0100	[thread overview]
Message-ID: <20221231150919.659533-6-agruenba@redhat.com> (raw)
In-Reply-To: <20221231150919.659533-1-agruenba@redhat.com>

Change the iomap ->page_prepare() handler to get and return a locked
folio instead of doing that in iomap_write_begin().  This allows to
recover from out-of-memory situations in ->page_prepare(), which
eliminates the corresponding error handling code in iomap_write_begin().
The ->put_folio() handler now also isn't called with NULL as the folio
value anymore.

Filesystems are expected to use the iomap_get_folio() helper for getting
locked folios in their ->page_prepare() handlers.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/bmap.c         | 21 +++++++++++++--------
 fs/iomap/buffered-io.c | 17 ++++++-----------
 include/linux/iomap.h  |  9 +++++----
 3 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 0c041459677b..41349e09558b 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -956,15 +956,25 @@ static int __gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 	goto out;
 }
 
-static int gfs2_iomap_page_prepare(struct inode *inode, loff_t pos,
-				   unsigned len)
+static struct folio *
+gfs2_iomap_page_prepare(struct iomap_iter *iter, loff_t pos, unsigned len)
 {
+	struct inode *inode = iter->inode;
 	unsigned int blockmask = i_blocksize(inode) - 1;
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	unsigned int blocks;
+	struct folio *folio;
+	int status;
 
 	blocks = ((pos & blockmask) + len + blockmask) >> inode->i_blkbits;
-	return gfs2_trans_begin(sdp, RES_DINODE + blocks, 0);
+	status = gfs2_trans_begin(sdp, RES_DINODE + blocks, 0);
+	if (status)
+		return ERR_PTR(status);
+
+	folio = iomap_get_folio(iter, pos);
+	if (IS_ERR(folio))
+		gfs2_trans_end(sdp);
+	return folio;
 }
 
 static void gfs2_iomap_put_folio(struct inode *inode, loff_t pos,
@@ -974,11 +984,6 @@ static void gfs2_iomap_put_folio(struct inode *inode, loff_t pos,
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 
-	if (!folio) {
-		gfs2_trans_end(sdp);
-		return;
-	}
-
 	if (!gfs2_is_stuffed(ip))
 		gfs2_page_add_databufs(ip, &folio->page, offset_in_page(pos),
 				       copied);
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index b84838d2b5d8..7decd8cdc755 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -609,7 +609,7 @@ static void iomap_put_folio(struct iomap_iter *iter, loff_t pos, size_t ret,
 
 	if (page_ops && page_ops->put_folio) {
 		page_ops->put_folio(iter->inode, pos, ret, folio);
-	} else if (folio) {
+	} else {
 		folio_unlock(folio);
 		folio_put(folio);
 	}
@@ -642,17 +642,12 @@ static int iomap_write_begin(struct iomap_iter *iter, loff_t pos,
 	if (!mapping_large_folio_support(iter->inode->i_mapping))
 		len = min_t(size_t, len, PAGE_SIZE - offset_in_page(pos));
 
-	if (page_ops && page_ops->page_prepare) {
-		status = page_ops->page_prepare(iter->inode, pos, len);
-		if (status)
-			return status;
-	}
-
-	folio = iomap_get_folio(iter, pos);
-	if (IS_ERR(folio)) {
-		iomap_put_folio(iter, pos, 0, NULL);
+	if (page_ops && page_ops->page_prepare)
+		folio = page_ops->page_prepare(iter, pos, len);
+	else
+		folio = iomap_get_folio(iter, pos);
+	if (IS_ERR(folio))
 		return PTR_ERR(folio);
-	}
 
 	/*
 	 * Now we have a locked folio, before we do anything with it we need to
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index e5732cc5716b..87b5d0f8e578 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -13,6 +13,7 @@
 struct address_space;
 struct fiemap_extent_info;
 struct inode;
+struct iomap_iter;
 struct iomap_dio;
 struct iomap_writepage_ctx;
 struct iov_iter;
@@ -131,12 +132,12 @@ static inline bool iomap_inline_data_valid(const struct iomap *iomap)
  * associated with them.
  *
  * When page_prepare succeeds, put_folio will always be called to do any
- * cleanup work necessary.  In that put_folio call, @folio will be NULL if the
- * associated folio could not be obtained.  When folio is not NULL, put_folio
- * is responsible for unlocking and putting the folio.
+ * cleanup work necessary.  put_folio is responsible for unlocking and putting
+ * @folio.
  */
 struct iomap_page_ops {
-	int (*page_prepare)(struct inode *inode, loff_t pos, unsigned len);
+	struct folio *(*page_prepare)(struct iomap_iter *iter, loff_t pos,
+			unsigned len);
 	void (*put_folio)(struct inode *inode, loff_t pos, unsigned copied,
 			struct folio *folio);
 
-- 
2.38.1


WARNING: multiple messages have this Message-ID (diff)
From: Andreas Gruenbacher <agruenba@redhat.com>
To: Christoph Hellwig <hch@infradead.org>,
	"Darrick J . Wong" <djwong@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Matthew Wilcox <willy@infradead.org>
Cc: Andreas Gruenbacher <agruenba@redhat.com>,
	linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org, cluster-devel@redhat.com
Subject: [PATCH v5 5/9] iomap/gfs2: Get page in page_prepare handler
Date: Sat, 31 Dec 2022 16:09:15 +0100	[thread overview]
Message-ID: <20221231150919.659533-6-agruenba@redhat.com> (raw)
In-Reply-To: <20221231150919.659533-1-agruenba@redhat.com>

Change the iomap ->page_prepare() handler to get and return a locked
folio instead of doing that in iomap_write_begin().  This allows to
recover from out-of-memory situations in ->page_prepare(), which
eliminates the corresponding error handling code in iomap_write_begin().
The ->put_folio() handler now also isn't called with NULL as the folio
value anymore.

Filesystems are expected to use the iomap_get_folio() helper for getting
locked folios in their ->page_prepare() handlers.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/bmap.c         | 21 +++++++++++++--------
 fs/iomap/buffered-io.c | 17 ++++++-----------
 include/linux/iomap.h  |  9 +++++----
 3 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 0c041459677b..41349e09558b 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -956,15 +956,25 @@ static int __gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 	goto out;
 }
 
-static int gfs2_iomap_page_prepare(struct inode *inode, loff_t pos,
-				   unsigned len)
+static struct folio *
+gfs2_iomap_page_prepare(struct iomap_iter *iter, loff_t pos, unsigned len)
 {
+	struct inode *inode = iter->inode;
 	unsigned int blockmask = i_blocksize(inode) - 1;
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	unsigned int blocks;
+	struct folio *folio;
+	int status;
 
 	blocks = ((pos & blockmask) + len + blockmask) >> inode->i_blkbits;
-	return gfs2_trans_begin(sdp, RES_DINODE + blocks, 0);
+	status = gfs2_trans_begin(sdp, RES_DINODE + blocks, 0);
+	if (status)
+		return ERR_PTR(status);
+
+	folio = iomap_get_folio(iter, pos);
+	if (IS_ERR(folio))
+		gfs2_trans_end(sdp);
+	return folio;
 }
 
 static void gfs2_iomap_put_folio(struct inode *inode, loff_t pos,
@@ -974,11 +984,6 @@ static void gfs2_iomap_put_folio(struct inode *inode, loff_t pos,
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 
-	if (!folio) {
-		gfs2_trans_end(sdp);
-		return;
-	}
-
 	if (!gfs2_is_stuffed(ip))
 		gfs2_page_add_databufs(ip, &folio->page, offset_in_page(pos),
 				       copied);
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index b84838d2b5d8..7decd8cdc755 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -609,7 +609,7 @@ static void iomap_put_folio(struct iomap_iter *iter, loff_t pos, size_t ret,
 
 	if (page_ops && page_ops->put_folio) {
 		page_ops->put_folio(iter->inode, pos, ret, folio);
-	} else if (folio) {
+	} else {
 		folio_unlock(folio);
 		folio_put(folio);
 	}
@@ -642,17 +642,12 @@ static int iomap_write_begin(struct iomap_iter *iter, loff_t pos,
 	if (!mapping_large_folio_support(iter->inode->i_mapping))
 		len = min_t(size_t, len, PAGE_SIZE - offset_in_page(pos));
 
-	if (page_ops && page_ops->page_prepare) {
-		status = page_ops->page_prepare(iter->inode, pos, len);
-		if (status)
-			return status;
-	}
-
-	folio = iomap_get_folio(iter, pos);
-	if (IS_ERR(folio)) {
-		iomap_put_folio(iter, pos, 0, NULL);
+	if (page_ops && page_ops->page_prepare)
+		folio = page_ops->page_prepare(iter, pos, len);
+	else
+		folio = iomap_get_folio(iter, pos);
+	if (IS_ERR(folio))
 		return PTR_ERR(folio);
-	}
 
 	/*
 	 * Now we have a locked folio, before we do anything with it we need to
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index e5732cc5716b..87b5d0f8e578 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -13,6 +13,7 @@
 struct address_space;
 struct fiemap_extent_info;
 struct inode;
+struct iomap_iter;
 struct iomap_dio;
 struct iomap_writepage_ctx;
 struct iov_iter;
@@ -131,12 +132,12 @@ static inline bool iomap_inline_data_valid(const struct iomap *iomap)
  * associated with them.
  *
  * When page_prepare succeeds, put_folio will always be called to do any
- * cleanup work necessary.  In that put_folio call, @folio will be NULL if the
- * associated folio could not be obtained.  When folio is not NULL, put_folio
- * is responsible for unlocking and putting the folio.
+ * cleanup work necessary.  put_folio is responsible for unlocking and putting
+ * @folio.
  */
 struct iomap_page_ops {
-	int (*page_prepare)(struct inode *inode, loff_t pos, unsigned len);
+	struct folio *(*page_prepare)(struct iomap_iter *iter, loff_t pos,
+			unsigned len);
 	void (*put_folio)(struct inode *inode, loff_t pos, unsigned copied,
 			struct folio *folio);
 
-- 
2.38.1


  parent reply	other threads:[~2022-12-31 15:09 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-31 15:09 [Cluster-devel] [PATCH v5 0/9] Turn iomap_page_ops into iomap_folio_ops Andreas Gruenbacher
2022-12-31 15:09 ` Andreas Gruenbacher
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 1/9] iomap: Add iomap_put_folio helper Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-04 17:39   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:39     ` Darrick J. Wong
2023-01-08 17:25   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:25     ` Christoph Hellwig
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 2/9] iomap/gfs2: Unlock and put folio in page_done handler Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-04 17:40   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:40     ` Darrick J. Wong
2023-01-08 17:26   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:26     ` Christoph Hellwig
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 3/9] iomap: Rename page_done handler to put_folio Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-04 17:37   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:37     ` Darrick J. Wong
2023-01-04 18:51     ` [Cluster-devel] " Andreas Grünbacher
2023-01-04 18:51       ` Andreas Grünbacher
2023-01-08 17:26   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:26     ` Christoph Hellwig
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 4/9] iomap: Add iomap_get_folio helper Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-04 17:38   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:38     ` Darrick J. Wong
2023-01-08 17:26   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:26     ` Christoph Hellwig
2022-12-31 15:09 ` Andreas Gruenbacher [this message]
2022-12-31 15:09   ` [PATCH v5 5/9] iomap/gfs2: Get page in page_prepare handler Andreas Gruenbacher
2023-01-04 17:45   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:45     ` Darrick J. Wong
2023-01-08 17:29   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:29     ` Christoph Hellwig
2023-01-08 19:40     ` [Cluster-devel] " Andreas Gruenbacher
2023-01-08 19:40       ` Andreas Gruenbacher
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 6/9] iomap: Rename page_prepare handler to get_folio Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-04 17:46   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:46     ` Darrick J. Wong
2023-01-08 17:31   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:31     ` Christoph Hellwig
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 7/9] iomap/xfs: Eliminate the iomap_valid handler Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-04 17:53   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:53     ` Darrick J. Wong
2023-01-04 19:02     ` [Cluster-devel] " Andreas Grünbacher
2023-01-04 19:02       ` Andreas Grünbacher
2023-01-04 19:08     ` [Cluster-devel] " Matthew Wilcox
2023-01-04 19:08       ` Matthew Wilcox
2023-01-08 17:32       ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:32         ` Christoph Hellwig
2023-01-08 18:50         ` [Cluster-devel] " Andreas Gruenbacher
2023-01-08 18:50           ` Andreas Gruenbacher
2023-01-10 21:56           ` [Cluster-devel] " Darrick J. Wong
2023-01-10 21:56             ` Darrick J. Wong
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 8/9] iomap: Rename page_ops to folio_ops Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-04 17:53   ` [Cluster-devel] " Darrick J. Wong
2023-01-04 17:53     ` Darrick J. Wong
2023-01-08 17:33   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:33     ` Christoph Hellwig
2022-12-31 15:09 ` [Cluster-devel] [PATCH v5 9/9] xfs: Make xfs_iomap_folio_ops static Andreas Gruenbacher
2022-12-31 15:09   ` Andreas Gruenbacher
2023-01-08 17:33   ` [Cluster-devel] " Christoph Hellwig
2023-01-08 17:33     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221231150919.659533-6-agruenba@redhat.com \
    --to=agruenba@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.