Re: [PATCH 08/12] iomap: move folio_unlock out of iomap_writeback_folio

Brian Foster
2025-06-27T16:35:24Z

On Fri, Jun 27, 2025 at 09:02:41AM +0200, Christoph Hellwig wrote:
> From: Joanne Koong <joannelkoong@gmail.com>
>
> Move unlocking the folio out of iomap_writeback_folio into the caller.
> This means the end writeback machinery is now run with the folio locked
> when no writeback happend, or writeback completed extremely fast.
>

I notice that folio_end_dropbehind_write() (via folio_end_writeback())
wants to trylock the folio in order to do its thing. Is this going to
cause issues with that (i.e. prevent invalidations)?

Brian

> This prepares for exporting iomap_writeback_folio for use in folio
> laundering.
>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> [hch: split from a larger patch]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/iomap/buffered-io.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index c28eb6a6eee4..a1dccf4e7063 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1656,10 +1656,8 @@ static int iomap_writeback_folio(struct iomap_writeback_ctx *wpc,
>
> trace_iomap_writeback_folio(inode, pos, folio_size(folio));
>
> - if (!iomap_writeback_handle_eof(folio, inode, &end_pos)) {
> - folio_unlock(folio);
> + if (!iomap_writeback_handle_eof(folio, inode, &end_pos))
> return 0;
> - }
> WARN_ON_ONCE(end_pos <= pos);
>
> if (i_blocks_per_folio(inode, folio) > 1) {
> @@ -1713,7 +1711,6 @@ static int iomap_writeback_folio(struct iomap_writeback_ctx *wpc,
> * already at this point. In that case we need to clear the writeback
> * bit ourselves right after unlocking the page.
> */
> - folio_unlock(folio);
> if (ifs) {
> if (atomic_dec_and_test(&ifs->write_bytes_pending))
> folio_end_writeback(folio);
> @@ -1740,8 +1737,10 @@ iomap_writepages(struct iomap_writeback_ctx *wpc)
> PF_MEMALLOC))
> return -EIO;
>
> - while ((folio = writeback_iter(mapping, wpc->wbc, folio, &error)))
> + while ((folio = writeback_iter(mapping, wpc->wbc, folio, &error))) {
> error = iomap_writeback_folio(wpc, folio);
> + folio_unlock(folio);
> + }
>
> /*
> * If @error is non-zero, it means that we have a situation where some
> --
> 2.47.2
>
>