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

Andreas Gruenbacher
2023-06-12T12:56:43Z

On Mon, Jun 12, 2023 at 8:25 AM Christoph Hellwig <hch@infradead.org> wrote:
> On Sat, Jun 10, 2023 at 05:09:04PM +0530, Ritesh Harjani (IBM) wrote:
> > This patch adds two of the helper routines iomap_ifs_is_fully_uptodate()
> > and iomap_ifs_is_block_uptodate() for managing uptodate state of
> > ifs state bitmap.
> >
> > In later patches ifs state bitmap array will also handle dirty state of all
> > blocks of a folio. Hence this patch adds some helper routines for handling
> > uptodate state of the ifs state bitmap.
> >
> > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > ---
> > fs/iomap/buffered-io.c | 28 ++++++++++++++++++++--------
> > 1 file changed, 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index e237f2b786bc..206808f6e818 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -43,6 +43,20 @@ static inline struct iomap_folio_state *iomap_get_ifs(struct folio *folio)
> >
> > static struct bio_set iomap_ioend_bioset;
> >
> > +static inline bool iomap_ifs_is_fully_uptodate(struct folio *folio,
> > + struct iomap_folio_state *ifs)
> > +{
> > + struct inode *inode = folio->mapping->host;
> > +
> > + return bitmap_full(ifs->state, i_blocks_per_folio(inode, folio));
> > +}
> > +
> > +static inline bool iomap_ifs_is_block_uptodate(struct iomap_folio_state *ifs,
> > + unsigned int block)
> > +{
> > + return test_bit(block, ifs->state);

"block_is_uptodate" instead of "is_block_uptodate" here as well, please.

Also see by previous mail about iomap_ifs_is_block_uptodate().

> > +}
>
> A little nitpicky, but do the _ifs_ name compenents here really add
> value?

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

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

iomap_ifs_set_range_uptodate -> __iomap_set_range_uptodate
iomap_ifs_clear_range_dirty -> __iomap_clear_range_dirty
iomap_ifs_set_range_dirty -> __iomap_set_range_dirty

Thanks,
Andreas