Re: [PATCH 00/21] Lock extents before pages

[Christoph Hellwig <hch@infradead.org>]

On Fri, Mar 10, 2023 at 10:38:28AM -0600, Goldwyn Rodrigues wrote:
> > I am really worried about the i_count hack.  It is fundamentlly wrong
> > and breaks inode lifetime rules.  Furthermore there is no attempt
> > at understanding why it happens (or even if it is new with this series).
> 
> So this (another version) patch which first added working with i_count is:
> 8180ef8894fa ("Btrfs: keep inode pinned when compressing writes")
> 
> For async, inode is still referenced until all the pages
> are written back and cleared in end_compressed_writeback().
> evict() waits for all writeback to be completed. Josef, is this
> correct?
> 
> I removed the ihold() and delayed iput sequence and shifted
> unlock_extent() immediately after submit_compressed_extents(). It is
> working fine with no crashes for compress tests. But since this is a
> race condition I need to be sure if it is the correct thing to do. I
> have updated the git [1] but it needs more testing.

I think the import thing to remember here is:

 1) an inode is not getting fred as long as it has pages under writeback
 2) as long as you don't reference the inode after clearing the last
    PageWriteback bit, using the inode without an extra reference
    in the writeback code is fine

So I think we can just remove the extra referene after making sure that
the inode is not referenced after dropping the last writeback bit.
And handling of the writeback bit is quite funny in btrfs in a few
places.  Maybe I'll be able to find some time to dig into it while
you're away.