Re: [PATCH v2 3/7] iomap: Remove unnecessary test from iomap_release_folio()

Matthew Wilcox
2023-06-04T21:39:16Z

On Sun, Jun 04, 2023 at 11:01:41AM -0700, Darrick J. Wong wrote:
> On Fri, Jun 02, 2023 at 11:24:40PM +0100, Matthew Wilcox (Oracle) wrote:
> > The check for the folio being under writeback is unnecessary; the caller
> > has checked this and the folio is locked, so the folio cannot be under
> > writeback at this point.
>
> Do we need a debug assertion here to validate that filemap_release_folio
> has already filtered out folios unergoing writeback? The documentation
> change in the next patch might be fine since you're the pagecache
> maintainer.

I don't think so? We don't usually include asserts in filesystems that
the VFS is living up to its promises.

> > /*
> > - * mm accommodates an old ext3 case where clean folios might
> > - * not have had the dirty bit cleared. Thus, it can send actual
> > - * dirty folios to ->release_folio() via shrink_active_list();
> > - * skip those here.
> > + * If the folio is dirty, we refuse to release our metadata because
> > + * it may be partially dirty (FIXME, add a test for that).
>
> Er... is this FIXME reflective of incomplete code?

It's a note to Ritesh ;-)

Once we have per-block dirty bits, if all the dirty bits are set, we
can free the iomap_page without losing any information. But I don't
want to introduce a dependency between this and Ritesh's work.