From: Andreas Gruenbacher <agruenba@redhat.com>
To: Christoph Hellwig <hch@infradead.org>,
"Darrick J. Wong" <djwong@kernel.org>
Cc: Andreas Gruenbacher <agruenba@redhat.com>,
linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: [PATCH v2] iomap: Move page_done callback under the folio lock
Date: Wed, 14 Dec 2022 11:24:09 +0100 [thread overview]
Message-ID: <20221214102409.1857526-1-agruenba@redhat.com> (raw)
In-Reply-To: <Y5l9zhhyOE+RNVgO@infradead.org>
Move the ->page_done() call in iomap_write_end() under the folio lock.
This closes a race between journaled data writes and the shrinker in
gfs2. What's happening is that gfs2_iomap_page_done() is called after
the page has been unlocked, so try_to_free_buffers() can come in and
free the buffers while gfs2_iomap_page_done() is trying to add them to
the current transaction. The folio lock prevents that from happening.
The only current user of ->page_done() is gfs2, so other filesystems are
not affected. Still, to catch out any new users, switch from page to
folio in ->page_done().
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/bmap.c | 7 ++++---
fs/iomap/buffered-io.c | 4 ++--
include/linux/iomap.h | 10 +++++-----
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index e7537fd305dd..c4ee47f8e499 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -968,14 +968,15 @@ static int gfs2_iomap_page_prepare(struct inode *inode, loff_t pos,
}
static void gfs2_iomap_page_done(struct inode *inode, loff_t pos,
- unsigned copied, struct page *page)
+ unsigned copied, struct folio *folio)
{
struct gfs2_trans *tr = current->journal_info;
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
- if (page && !gfs2_is_stuffed(ip))
- gfs2_page_add_databufs(ip, page, offset_in_page(pos), copied);
+ if (folio && !gfs2_is_stuffed(ip))
+ gfs2_page_add_databufs(ip, &folio->page, offset_in_page(pos),
+ copied);
if (tr->tr_num_buf_new)
__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 91ee0b308e13..d988c1bedf70 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -714,12 +714,12 @@ static size_t iomap_write_end(struct iomap_iter *iter, loff_t pos, size_t len,
i_size_write(iter->inode, pos + ret);
iter->iomap.flags |= IOMAP_F_SIZE_CHANGED;
}
+ if (page_ops && page_ops->page_done)
+ page_ops->page_done(iter->inode, pos, ret, folio);
folio_unlock(folio);
if (old_size < pos)
pagecache_isize_extended(iter->inode, old_size, pos);
- if (page_ops && page_ops->page_done)
- page_ops->page_done(iter->inode, pos, ret, &folio->page);
folio_put(folio);
if (ret < len)
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 238a03087e17..bd6d80453726 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -116,18 +116,18 @@ static inline bool iomap_inline_data_valid(const struct iomap *iomap)
/*
* When a filesystem sets page_ops in an iomap mapping it returns, page_prepare
- * and page_done will be called for each page written to. This only applies to
- * buffered writes as unbuffered writes will not typically have pages
+ * and page_done will be called for each folio written to. This only applies
+ * to buffered writes as unbuffered writes will not typically have folios
* associated with them.
*
* When page_prepare succeeds, page_done will always be called to do any
- * cleanup work necessary. In that page_done call, @page will be NULL if the
- * associated page could not be obtained.
+ * cleanup work necessary. In that page_done call, @folio will be NULL if the
+ * associated folio could not be obtained.
*/
struct iomap_page_ops {
int (*page_prepare)(struct inode *inode, loff_t pos, unsigned len);
void (*page_done)(struct inode *inode, loff_t pos, unsigned copied,
- struct page *page);
+ struct folio *folio);
};
/*
--
2.38.1
next prev parent reply other threads:[~2022-12-14 10:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-13 19:48 [PATCH] iomap: Move page_done callback under the folio lock Andreas Gruenbacher
2022-12-13 20:03 ` Darrick J. Wong
2022-12-13 20:15 ` Andreas Gruenbacher
2022-12-14 7:39 ` Christoph Hellwig
2022-12-14 10:23 ` Andreas Gruenbacher
2022-12-14 10:24 ` Andreas Gruenbacher [this message]
2022-12-15 20:13 ` [PATCH v2] " Andreas Gruenbacher
2022-12-16 8:22 ` 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=20221214102409.1857526-1-agruenba@redhat.com \
--to=agruenba@redhat.com \
--cc=djwong@kernel.org \
--cc=hch@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.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.