On Mon, Jun 05, 2023 at 04:15:31PM +0200, Andreas Gruenbacher wrote:
> Note that to_iomap_page() does folio_test_private() followed by
> folio_get_private(), which doesn't really make sense in places where
> we know that iop is defined. Maybe we want to split that up.
The plan is to retire the folio private flag entirely. I just haven't
got round to cleaning up iomap yet. For folios which we know to be
file-backed, we can just test whether folio->private is NULL or not.
So I'd do this as:
- struct iomap_page *iop = to_iomap_page(folio);
+ struct iomap_page *iop = folio->private;
and not even use folio_get_private() or to_iomap_page() any more.
> > + unsigned int first_blk = off >> inode->i_blkbits;
> > + unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
> > + unsigned int nr_blks = last_blk - first_blk + 1;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&iop->state_lock, flags);
> > + bitmap_set(iop->state, first_blk, nr_blks);
> > + if (iop_test_full_uptodate(folio))
> > + folio_mark_uptodate(folio);
> > + spin_unlock_irqrestore(&iop->state_lock, flags);
> > +}
> > +
> > +static void iomap_iop_set_range_uptodate(struct inode *inode,
> > + struct folio *folio, size_t off, size_t len)
> > +{
> > + struct iomap_page *iop = to_iomap_page(folio);
> > +
> > + if (iop)
> > + iop_set_range_uptodate(inode, folio, off, len);
> > + else
> > + folio_mark_uptodate(folio);
> > +}
>
> This patch passes the inode into iomap_iop_set_range_uptodate() and
> removes the iop argument. Can this be done in a separate patch,
> please?
>
> We have a few places like the above, where we look up the iop without
> using it. Would a function like folio_has_iop() make more sense?
I think this is all a symptom of the unnecessary splitting of
iomap_iop_set_range_uptodate and iop_set_range_uptodate.
