On Tue, Jun 06, 2023 at 06:30:35PM +0530, Ritesh Harjani wrote: > Matthew Wilcox <willy@xxxxxxxxxxxxx> 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
