Am Mo., 12. Juni 2023 um 17:43 Uhr schrieb Ritesh Harjani
<ritesh.list@xxxxxxxxx>:
> Andreas Gruenbacher <agruenba@xxxxxxxxxx> writes:
> > On Sat, Jun 10, 2023 at 1:39 PM Ritesh Harjani (IBM)
> > <ritesh.list@xxxxxxxxx> 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@xxxxxxxxx>
> >> ---
> >> 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));
> >
> > This should be written as something like:
> >
> > unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
> > return bitmap_full(ifs->state + IOMAP_ST_UPTODATE * blks_per_folio,
> > blks_per_folio);
> >
>
> Nah, I feel it is not required... It make sense when we have the same
> function getting use for both "uptodate" and "dirty" state.
> Here the function anyways operates on uptodate state.
> Hence I feel it is not required.
So we have this iomap_block_state enum now, but IOMAP_ST_UPTODATE must
be 0 or else the code will break. That's worse than not having this
abstraction in the first place because.
Andreas
> >> +}
> >> +
> >> +static inline bool iomap_ifs_is_block_uptodate(struct iomap_folio_state *ifs,
> >> + unsigned int block)
> >> +{
> >> + return test_bit(block, ifs->state);
> >
> > This function should be called iomap_ifs_block_is_uptodate(), and
> > probably be written as follows, passing in the folio as well (this
> > will optimize out, anyway):
> >
> > struct inode *inode = folio->mapping->host;
> > unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
> > return test_bit(block, ifs->state + IOMAP_ST_UPTODATE * blks_per_folio);
> >
>
> Same here.
>
> -ritesh
