From 80baab88bb93eeaa133b426d24dfc0775a8cf824 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 15 Jan 2023 08:45:51 -0800 Subject: iomap/gfs2: Unlock and put folio in page_done handler When an iomap defines a ->page_done() handler in its page_ops, delegate unlocking the folio and putting the folio reference to that handler. This allows to fix a race between journaled data writes and folio writeback in gfs2: before this change, gfs2_iomap_page_done() was called after unlocking the folio, so writeback could start writing back the folio's buffers before they could be marked for writing to the journal. Also, try_to_free_buffers() could free the buffers before gfs2_iomap_page_done() was done adding the buffers to the current current transaction. With this change, gfs2_iomap_page_done() adds the buffers to the current transaction while the folio is still locked, so the problems described above can no longer occur. The only current user of ->page_done() is gfs2, so other filesystems are not affected. To catch out any out-of-tree users, switch from a page to a folio in ->page_done(). Signed-off-by: Andreas Gruenbacher Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Signed-off-by: Darrick J. Wong --- include/linux/iomap.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'include/linux') diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 0983dfc9a203..743e2a909162 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -131,13 +131,14 @@ 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, @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. When folio is not NULL, page_done + * is responsible for unlocking and putting the folio. */ 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); /* * Check that the cached iomap still maps correctly to the filesystem's -- cgit v1.2.3 From 40405dddd98a9a95585482af46b8e269f5ebe5df Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 15 Jan 2023 08:45:51 -0800 Subject: iomap: Rename page_done handler to put_folio The ->page_done() handler in struct iomap_page_ops is now somewhat misnamed in that it mainly deals with unlocking and putting a folio, so rename it to ->put_folio(). Signed-off-by: Andreas Gruenbacher Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Signed-off-by: Darrick J. Wong --- include/linux/iomap.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'include/linux') diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 743e2a909162..ecf815b34d51 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -126,18 +126,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 put_folio 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, @folio will be NULL if the - * associated folio could not be obtained. When folio is not NULL, page_done + * 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. */ 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, + void (*put_folio)(struct inode *inode, loff_t pos, unsigned copied, struct folio *folio); /* -- cgit v1.2.3 From 98321b5139f92a736a426404fb6e23bfb8feb9cc Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 15 Jan 2023 08:49:04 -0800 Subject: iomap: Add iomap_get_folio helper Add an iomap_get_folio() helper that gets a folio reference based on an iomap iterator and an offset into the address space. Use it in iomap_write_begin(). Signed-off-by: Andreas Gruenbacher Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Signed-off-by: Darrick J. Wong --- include/linux/iomap.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/linux') diff --git a/include/linux/iomap.h b/include/linux/iomap.h index ecf815b34d51..188d14e786a4 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -261,6 +261,7 @@ int iomap_file_buffered_write_punch_delalloc(struct inode *inode, int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops); void iomap_readahead(struct readahead_control *, const struct iomap_ops *ops); bool iomap_is_partially_uptodate(struct folio *, size_t from, size_t count); +struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos); bool iomap_release_folio(struct folio *folio, gfp_t gfp_flags); void iomap_invalidate_folio(struct folio *folio, size_t offset, size_t len); int iomap_file_unshare(struct inode *inode, loff_t pos, loff_t len, -- cgit v1.2.3 From 9060bc4d3aca6106bbe72891efba391d9d6b86e7 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 15 Jan 2023 08:49:12 -0800 Subject: iomap/gfs2: Get page in page_prepare handler 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 Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Signed-off-by: Darrick J. Wong --- include/linux/iomap.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'include/linux') diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 188d14e786a4..d50501781856 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); -- cgit v1.2.3 From c82abc2394642490da736b1ba78671bbcef81150 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 15 Jan 2023 08:50:25 -0800 Subject: iomap: Rename page_prepare handler to get_folio The ->page_prepare() handler in struct iomap_page_ops is now somewhat misnamed, so rename it to ->get_folio(). Signed-off-by: Andreas Gruenbacher Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Signed-off-by: Darrick J. Wong --- include/linux/iomap.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'include/linux') diff --git a/include/linux/iomap.h b/include/linux/iomap.h index d50501781856..da226032aedc 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -126,17 +126,17 @@ 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 + * When a filesystem sets page_ops in an iomap mapping it returns, get_folio * and put_folio 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, put_folio will always be called to do any + * When get_folio succeeds, put_folio will always be called to do any * cleanup work necessary. put_folio is responsible for unlocking and putting * @folio. */ struct iomap_page_ops { - struct folio *(*page_prepare)(struct iomap_iter *iter, loff_t pos, + struct folio *(*get_folio)(struct iomap_iter *iter, loff_t pos, unsigned len); void (*put_folio)(struct inode *inode, loff_t pos, unsigned copied, struct folio *folio); -- cgit v1.2.3 From 471859f57d42537626a56312cfb50cd6acee09ae Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Sun, 15 Jan 2023 08:50:44 -0800 Subject: iomap: Rename page_ops to folio_ops The operations in struct page_ops all operate on folios, so rename struct page_ops to struct folio_ops. Signed-off-by: Andreas Gruenbacher Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig [djwong: port around not removing iomap_valid] Signed-off-by: Darrick J. Wong --- include/linux/iomap.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'include/linux') diff --git a/include/linux/iomap.h b/include/linux/iomap.h index da226032aedc..c70a9d4fb447 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -86,7 +86,7 @@ struct vm_fault; */ #define IOMAP_NULL_ADDR -1ULL /* addr is not valid */ -struct iomap_page_ops; +struct iomap_folio_ops; struct iomap { u64 addr; /* disk offset of mapping, bytes */ @@ -98,7 +98,7 @@ struct iomap { struct dax_device *dax_dev; /* dax_dev for dax operations */ void *inline_data; void *private; /* filesystem private */ - const struct iomap_page_ops *page_ops; + const struct iomap_folio_ops *folio_ops; u64 validity_cookie; /* used with .iomap_valid() */ }; @@ -126,7 +126,7 @@ static inline bool iomap_inline_data_valid(const struct iomap *iomap) } /* - * When a filesystem sets page_ops in an iomap mapping it returns, get_folio + * When a filesystem sets folio_ops in an iomap mapping it returns, get_folio * and put_folio 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. @@ -135,7 +135,7 @@ static inline bool iomap_inline_data_valid(const struct iomap *iomap) * cleanup work necessary. put_folio is responsible for unlocking and putting * @folio. */ -struct iomap_page_ops { +struct iomap_folio_ops { struct folio *(*get_folio)(struct iomap_iter *iter, loff_t pos, unsigned len); void (*put_folio)(struct inode *inode, loff_t pos, unsigned copied, -- cgit v1.2.3