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

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

Matthew Wilcox <willy@infradead.org> writes:

> 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.

Sure Matthew, noted.
However this looks more like an optimization which can be done for cases
where we have all bits as dirty, than a dependency.

-ritesh