Re: [PATCHv8 5/5] iomap: Add per-block dirty state tracking to improve performance

[Christoph Hellwig <hch@infradead.org>]

> +static inline bool iomap_iof_is_block_dirty(struct folio *folio,
> +			struct iomap_folio *iof, int block)

Two tabs indents here please and for various other functions.

> +{
> +	struct inode *inode = folio->mapping->host;
> +	unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
> +	unsigned int first_blk = off >> inode->i_blkbits;
> +	unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
> +	unsigned int nr_blks = last_blk - first_blk + 1;

Given how many places we we opencode this logic I wonder if a helper
would be usefuļ, even if the calling conventions are a bit odd.

To make this nicer it would also be good an take a neat trick from
the btrfs subpage support and use an enum for the bits, e.g.:

enum iomap_block_state {
	IOMAP_ST_UPTODATE,
	IOMAP_ST_DIRTY,

	IOMAP_ST_MAX,
};


static void iomap_ibs_calc_range(struct folio *folio, size_t off, size_t len,
		enum iomap_block_state state, unsigned int *first_blk,
		unsigned int *nr_blks)
{
	struct inode *inode = folio->mapping->host;
	unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
	unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;

	*first_blk = state * blks_per_folio + (off >> inode->i_blkbits);
	*nr_blks = last_blk - first_blk + 1;
}

> +	/*
> +	 * iof->state tracks two sets of state flags when the
> +	 * filesystem block size is smaller than the folio size.
> +	 * The first state tracks per-block uptodate and the
> +	 * second tracks per-block dirty state.
> +	 */
> +	iof = kzalloc(struct_size(iof, state, BITS_TO_LONGS(2 * nr_blocks)),
>  		      gfp);

with the above this can use IOMAP_ST_MAX and make the whole thing a
little more robus.

>
>  	if (iof) {

No that this adds a lot more initialization I'd do a

	if (!iof)
		return NULL;

here and unindent the rest.