Re: [PATCHv8 0/5] iomap: Add support for per-block dirty state to improve write performance

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 06, 2023 at 06:30:35PM +0530, Ritesh Harjani wrote:
> Matthew Wilcox <willy@infradead.org> writes:
> 
> > On Tue, Jun 06, 2023 at 05:13:47PM +0530, Ritesh Harjani (IBM) wrote:
> >> Hello All,
> >> 
> >> Please find PATCHv8 which adds per-block dirty tracking to iomap.
> >> As discussed earlier this is required to improve write performance and reduce
> >> write amplification for cases where either blocksize is less than pagesize (such
> >> as Power platform with 64k pagesize) or when we have a large folio (such as xfs
> >> which currently supports large folio).
> >
> > You're moving too fast.  Please, allow at least a few hours between
> > getting review comments and sending a new version.
> >
> 
> Sorry about that. I felt those were mainly only mechanical conversion
> changes. Will keep in mind.
> 
> >> v7 -> v8
> >> ==========
> >> 1. Renamed iomap_page -> iomap_folio & iop -> iof in Patch-1 itself.
> >
> > I don't think iomap_folio is the right name.  Indeed, I did not believe
> > that iomap_page was the right name.  As I said on #xfs recently ...
> >
> > <willy> i'm still not crazy about iomap_page as the name of that
> >    data structure.  and calling the variable 'iop' just seems doomed
> >    to be trouble.  how do people feel about either iomap_block_state or
> >    folio_block_state ... or even just calling it block_state since it's
> >    local to iomap/buffered-io.c
> > <willy> we'd then call the variable either ibs or fbs, both of which
> >    have some collisions in the kernel, but none in filesystems
> > <dchinner> willy - sounds reasonable
> 
> Both seems equally reasonable to me. If others are equally ok with both,
> then shall we go with iomap_block_state and ibs? 

I've a slight preference for struct folio_block_state/folio_bs/fbs.

Or even struct iomap_folio_state/iofs/ifs.

IBS is an acronym for a rather uncomfortable medical condition... ;)

--D

> I see that as "iomap_block_state" which is local to iomap buffered-io
> layer to track per-block state within a folio and gets attached to
> folio->private.
> 
> -ritesh