| Message ID | 20230602222445.2284892-2-willy@infradead.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Create large folios in iomap buffered write path | expand |
On Fri, Jun 02, 2023 at 11:24:38PM +0100, Matthew Wilcox (Oracle) wrote: > We do not need to release the iomap_page in iomap_invalidate_folio() > to allow the folio to be split. The splitting code will call > ->release_folio() if there is still per-fs private data attached to > the folio. At that point, we will check if the folio is still dirty > and decline to release the iomap_page. It is possible to trigger the > warning in perfectly legitimate circumstances (eg if a disk read fails, > we do a partial write to the folio, then we truncate the folio), which > will cause those writes to be lost. > > Fixes: 60d8231089f0 ("iomap: Support large folios in invalidatepage") > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Sounds reasonable to me... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/iomap/buffered-io.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 063133ec77f4..08ee293c4117 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -508,11 +508,6 @@ void iomap_invalidate_folio(struct folio *folio, size_t offset, size_t len) > WARN_ON_ONCE(folio_test_writeback(folio)); > folio_cancel_dirty(folio); > iomap_page_release(folio); > - } else if (folio_test_large(folio)) { > - /* Must release the iop so the page can be split */ > - WARN_ON_ONCE(!folio_test_uptodate(folio) && > - folio_test_dirty(folio)); > - iomap_page_release(folio); > } > } > EXPORT_SYMBOL_GPL(iomap_invalidate_folio); > -- > 2.39.2 >
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 063133ec77f4..08ee293c4117 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -508,11 +508,6 @@ void iomap_invalidate_folio(struct folio *folio, size_t offset, size_t len) WARN_ON_ONCE(folio_test_writeback(folio)); folio_cancel_dirty(folio); iomap_page_release(folio); - } else if (folio_test_large(folio)) { - /* Must release the iop so the page can be split */ - WARN_ON_ONCE(!folio_test_uptodate(folio) && - folio_test_dirty(folio)); - iomap_page_release(folio); } } EXPORT_SYMBOL_GPL(iomap_invalidate_folio);
We do not need to release the iomap_page in iomap_invalidate_folio() to allow the folio to be split. The splitting code will call ->release_folio() if there is still per-fs private data attached to the folio. At that point, we will check if the folio is still dirty and decline to release the iomap_page. It is possible to trigger the warning in perfectly legitimate circumstances (eg if a disk read fails, we do a partial write to the folio, then we truncate the folio), which will cause those writes to be lost. Fixes: 60d8231089f0 ("iomap: Support large folios in invalidatepage") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/iomap/buffered-io.c | 5 ----- 1 file changed, 5 deletions(-)