Re: [PATCH] iomap: Address soft lockup in iomap_finish_ioend()

["hch@infradead.org" <hch@infradead.org>]

On Tue, Jan 04, 2022 at 12:22:15PM +1100, Dave Chinner wrote:
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1098,6 +1098,15 @@ iomap_ioend_can_merge(struct iomap_ioend *ioend, struct iomap_ioend *next)
>  		return false;
>  	if (ioend->io_offset + ioend->io_size != next->io_offset)
>  		return false;
> +	/*
> +	 * Do not merge physically discontiguous ioends. The filesystem
> +	 * completion functions will have to iterate the physical
> +	 * discontiguities even if we merge the ioends at a logical level, so
> +	 * we don't gain anything by merging physical discontiguities here.
> +	 */
> +	if (ioend->io_inline_bio.bi_iter.bi_sector + (ioend->io_size >> 9) !=

This open codes bio_end_sector()

> +	    next->io_inline_bio.bi_iter.bi_sector)

But more importantly I don't think just using the inline_bio makes sense
here as the ioend can have multiple bios.  Fortunately we should always
have the last built bio available in ->io_bio.

> +		return false;
>  	return true;
>  }
>  
> @@ -1241,6 +1250,13 @@ iomap_can_add_to_ioend(struct iomap_writepage_ctx *wpc, loff_t offset,
>  		return false;
>  	if (sector != bio_end_sector(wpc->ioend->io_bio))
>  		return false;
> +	/*
> +	 * Limit ioend bio chain lengths to minimise IO completion latency. This
> +	 * also prevents long tight loops ending page writeback on all the pages
> +	 * in the ioend.
> +	 */
> +	if (wpc->ioend->io_size >= 4096 * PAGE_SIZE)
> +		return false;

And this stops making sense with the impending additions of large folio
support.  I think we need to count the pages/folios instead as the
operations are once per page/folio.