All of lore.kernel.org
 help / color / mirror / Atom feed
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: [RFC v3 5/7] iomap: Get page in page_prepare handler
Date: Fri, 16 Dec 2022 16:06:24 +0100	[thread overview]
Message-ID: <20221216150626.670312-6-agruenba@redhat.com> (raw)
In-Reply-To: <20221216150626.670312-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 ->page_done() handler is now not called with a NULL folio anymore.

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

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

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 11115fce68cb..cd5984d3ba50 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -959,15 +959,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_folio_prepare(iter, pos);
+	if (!folio)
+		gfs2_trans_end(sdp);
+	return folio;
 }
 
 static void gfs2_iomap_page_done(struct inode *inode, loff_t pos,
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index f0f167ca1b2e..6b7c1a10b8ec 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -602,7 +602,7 @@ static void iomap_folio_done(struct iomap_iter *iter, loff_t pos, size_t ret,
 
 	if (page_ops && page_ops->page_done) {
 		page_ops->page_done(iter->inode, pos, ret, folio);
-	} else if (folio) {
+	} else {
 		folio_unlock(folio);
 		folio_put(folio);
 	}
@@ -635,17 +635,14 @@ 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_folio_prepare(iter, pos);
-	if (!folio) {
-		status = (iter->flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOMEM;
-		iomap_folio_done(iter, pos, 0, NULL);
-		return status;
+	if (page_ops && page_ops->page_prepare)
+		folio = page_ops->page_prepare(iter, pos, len);
+	else
+		folio = iomap_folio_prepare(iter, pos);
+	if (IS_ERR_OR_NULL(folio)) {
+		if (!folio)
+			return (iter->flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOMEM;
+		return PTR_ERR(folio);
 	}
 
 	/*
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 0bf16ef22d81..c74ab8c53b47 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, page_done will always be called to do any
- * cleanup work necessary.  In that page_done call, @folio will be NULL if the
- * associated folio could not be obtained.  When folio is not NULL, page_done
- * is responsible for unlocking and putting the folio.
+ * cleanup work necessary.  page_done 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 (*page_done)(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: cluster-devel.redhat.com
Subject: [Cluster-devel] [RFC v3 5/7] iomap: Get page in page_prepare handler
Date: Fri, 16 Dec 2022 16:06:24 +0100	[thread overview]
Message-ID: <20221216150626.670312-6-agruenba@redhat.com> (raw)
In-Reply-To: <20221216150626.670312-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 ->page_done() handler is now not called with a NULL folio anymore.

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

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

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 11115fce68cb..cd5984d3ba50 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -959,15 +959,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_folio_prepare(iter, pos);
+	if (!folio)
+		gfs2_trans_end(sdp);
+	return folio;
 }
 
 static void gfs2_iomap_page_done(struct inode *inode, loff_t pos,
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index f0f167ca1b2e..6b7c1a10b8ec 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -602,7 +602,7 @@ static void iomap_folio_done(struct iomap_iter *iter, loff_t pos, size_t ret,
 
 	if (page_ops && page_ops->page_done) {
 		page_ops->page_done(iter->inode, pos, ret, folio);
-	} else if (folio) {
+	} else {
 		folio_unlock(folio);
 		folio_put(folio);
 	}
@@ -635,17 +635,14 @@ 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_folio_prepare(iter, pos);
-	if (!folio) {
-		status = (iter->flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOMEM;
-		iomap_folio_done(iter, pos, 0, NULL);
-		return status;
+	if (page_ops && page_ops->page_prepare)
+		folio = page_ops->page_prepare(iter, pos, len);
+	else
+		folio = iomap_folio_prepare(iter, pos);
+	if (IS_ERR_OR_NULL(folio)) {
+		if (!folio)
+			return (iter->flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOMEM;
+		return PTR_ERR(folio);
 	}
 
 	/*
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 0bf16ef22d81..c74ab8c53b47 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, page_done will always be called to do any
- * cleanup work necessary.  In that page_done call, @folio will be NULL if the
- * associated folio could not be obtained.  When folio is not NULL, page_done
- * is responsible for unlocking and putting the folio.
+ * cleanup work necessary.  page_done 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 (*page_done)(struct inode *inode, loff_t pos, unsigned copied,
 			struct folio *folio);
 
-- 
2.38.1


  parent reply	other threads:[~2022-12-16 15:08 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-16 15:06 [RFC v3 0/7] Turn iomap_page_ops into iomap_folio_ops Andreas Gruenbacher
2022-12-16 15:06 ` [Cluster-devel] " Andreas Gruenbacher
2022-12-16 15:06 ` [RFC v3 1/7] fs: Add folio_may_straddle_isize helper Andreas Gruenbacher
2022-12-16 15:06   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-23 14:56   ` Christoph Hellwig
2022-12-23 14:56     ` [Cluster-devel] " Christoph Hellwig
2022-12-23 22:04     ` Andreas Grünbacher
2022-12-23 22:04       ` [Cluster-devel] " Andreas Grünbacher
2022-12-24  7:21       ` Christoph Hellwig
2022-12-16 15:06 ` [RFC v3 2/7] iomap: Add iomap_folio_done helper Andreas Gruenbacher
2022-12-16 15:06   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-23 15:02   ` Christoph Hellwig
2022-12-23 15:02     ` [Cluster-devel] " Christoph Hellwig
2022-12-23 20:54     ` Andreas Grünbacher
2022-12-23 20:54       ` [Cluster-devel] " Andreas Grünbacher
2022-12-24  7:22       ` Christoph Hellwig
2022-12-24  7:22         ` [Cluster-devel] " Christoph Hellwig
2022-12-16 15:06 ` [RFC v3 3/7] iomap/gfs2: Unlock and put folio in page_done handler Andreas Gruenbacher
2022-12-16 15:06   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-23 15:03   ` Christoph Hellwig
2022-12-23 15:03     ` [Cluster-devel] " Christoph Hellwig
2022-12-16 15:06 ` [RFC v3 4/7] iomap: Add iomap_folio_prepare helper Andreas Gruenbacher
2022-12-16 15:06   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-23 15:04   ` Christoph Hellwig
2022-12-23 15:04     ` [Cluster-devel] " Christoph Hellwig
2022-12-23 21:05     ` Andreas Grünbacher
2022-12-23 21:05       ` [Cluster-devel] " Andreas Grünbacher
2022-12-24  7:23       ` Christoph Hellwig
2022-12-24  7:23         ` [Cluster-devel] " Christoph Hellwig
2022-12-25  9:12         ` Matthew Wilcox
2022-12-25  9:12           ` [Cluster-devel] " Matthew Wilcox
2022-12-28 15:55           ` Christoph Hellwig
2022-12-28 15:55             ` [Cluster-devel] " Christoph Hellwig
2022-12-16 15:06 ` Andreas Gruenbacher [this message]
2022-12-16 15:06   ` [Cluster-devel] [RFC v3 5/7] iomap: Get page in page_prepare handler Andreas Gruenbacher
2022-12-16 16:30   ` Matthew Wilcox
2022-12-16 16:30     ` [Cluster-devel] " Matthew Wilcox
2022-12-16 17:15     ` Andreas Gruenbacher
2022-12-16 17:15       ` [Cluster-devel] " Andreas Gruenbacher
2022-12-23 15:07   ` Christoph Hellwig
2022-12-23 15:07     ` [Cluster-devel] " Christoph Hellwig
2022-12-16 15:06 ` [RFC v3 6/7] iomap/xfs: Eliminate the iomap_valid handler Andreas Gruenbacher
2022-12-16 15:06   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-23 15:10   ` Christoph Hellwig
2022-12-23 15:10     ` [Cluster-devel] " Christoph Hellwig
2022-12-16 15:06 ` [RFC v3 7/7] iomap: Rename page_ops to folio_ops Andreas Gruenbacher
2022-12-16 15:06   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 0/7] Turn iomap_page_ops into iomap_folio_ops Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 1/7] fs: Add folio_may_straddle_isize helper Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 2/7] iomap: Add iomap_folio_done helper Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 3/7] iomap/gfs2: Unlock and put folio in page_done handler Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 4/7] iomap: Add iomap_folio_prepare helper Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 5/7] iomap/gfs2: Get page in page_prepare handler Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 6/7] iomap/xfs: Eliminate the iomap_valid handler Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher
2022-12-18 22:10 ` [RFC v4 7/7] iomap: Rename page_ops to folio_ops Andreas Gruenbacher
2022-12-18 22:10   ` [Cluster-devel] " Andreas Gruenbacher

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=20221216150626.670312-6-agruenba@redhat.com \
    --to=agruenba@redhat.com \
    --cc=cluster-devel@redhat.com \
    --cc=djwong@kernel.org \
    --cc=hch@infradead.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /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.