Re: [PATCHv9 3/6] iomap: Add some uptodate state handling helpers for ifs state bitmap

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

"Darrick J. Wong" <djwong@kernel.org> writes:

> On Mon, Jun 12, 2023 at 05:57:48PM +0200, Andreas Gruenbacher wrote:
>> On Mon, Jun 12, 2023 at 5:24 PM Matthew Wilcox <willy@infradead.org> wrote:
>> > On Mon, Jun 12, 2023 at 08:48:16PM +0530, Ritesh Harjani wrote:
>> > > > Since we're at the nitpicking, I don't find those names very useful,
>> > > > either. How about the following instead?
>> > > >
>> > > > iomap_ifs_alloc -> iomap_folio_state_alloc
>> > > > iomap_ifs_free -> iomap_folio_state_free
>> > > > iomap_ifs_calc_range -> iomap_folio_state_calc_range
>> > >
>> > > First of all I think we need to get used to the name "ifs" like how we
>> > > were using "iop" earlier. ifs == iomap_folio_state...
>> > >
>> > > >
>> > > > iomap_ifs_is_fully_uptodate -> iomap_folio_is_fully_uptodate
>> > > > iomap_ifs_is_block_uptodate -> iomap_block_is_uptodate
>> > > > iomap_ifs_is_block_dirty -> iomap_block_is_dirty
>> > > >
>> > >
>> > > ...if you then look above functions with _ifs_ == _iomap_folio_state_
>> > > naming. It will make more sense.
>> >
>> > Well, it doesn't because it's iomap_iomap_folio_state_is_fully_uptodate.
>> 
>> Exactly.
>> 
>> > I don't think there's any need to namespace this so fully.
>> > ifs_is_fully_uptodate() is just fine for a static function, IMO.
>> 
>> I'd be perfectly happy with that kind of naming scheme as well.
>
> Ugh, /another/ round of renaming.
>
> to_folio_state (or just folio->private)
>
> ifs_alloc
> ifs_free
> ifs_calc_range
>
> ifs_set_range_uptodate
> ifs_is_fully_uptodate
> ifs_block_is_uptodate
>
> ifs_block_is_dirty
> ifs_clear_range_dirty
> ifs_set_range_dirty
>

Oops you have put me into a tough spot here. 
We came back from iop_** functions naming to iomap_iop_** to
iomap_ifs_**.

Christoph? Is it ok if we go back to ifs_** functions here then? 

Or do others prefer 
iomap_folio_state_** namings. instead of ifs_**  or iomap_ifs_**? 


> No more renaming suggestions.  I've reached the point where my eyes and
> brain have both glazed over from repeated re-reads of this series such
> that I don't see the *bugs* anymore.
>
> Anyone else wanting new naming gets to *send in their own patch*.
> Please focus only on finding code defects or friction between iomap and
> some other subsystem.

Yes, it would be helpful if we uncover any bugs/ or even suggstions for
how can we better test this (adding/improving any given test in xfstests).

I have been using xfstests mainly on x86 with 1k and Power with 4k "-g auto".
I will make sure I run some more configs before sending the next
revision. 

>
> Flame away about my aggressive tone,

Thanks Darrick. No issues at all. 

>
> ~Your burned out and pissed off maintainer~
>
>> Thanks,
>> Andreas
>> 

-ritesh