Re: [PATCH 3/3] btrfs: don't hold an extra reference for redirtied buffers

[Christoph Hellwig <hch@lst.de>]

On Wed, May 10, 2023 at 12:57:37AM +0200, David Sterba wrote:
> On Mon, May 08, 2023 at 07:58:39AM -0700, Christoph Hellwig wrote:
> > When btrfs_redirty_list_add redirties a buffer, it also acquires
> > an extra reference that is released on transaction commit.  But
> > this is not required as buffers that are dirty or under writeback
> > are never freed (look for calls to extent_buffer_under_io())).
> > 
> > Remove the extra reference and the infrastructure used to drop it
> > again.
> 
> I vaguely remember that the redirty list was need for zoned to avoid
> some write pattern that disrupts the ordering, added in d3575156f662
> ("btrfs: zoned: redirty released extent buffers").

So the redirting itself is needed for that - without it buffers where
the dirty bit wasn't ever set would never get written, leading to a
write outside of the zone pointer.  But the extra reference can't
influece the write pattern, as we don't make writeback descriptions
based of it.

> I'd appreciate more eyes on this patch, with the indirections and
> writeback involved it's not clear to me that we don't need the list at
> all.

My suspicision is that Aoto-san wanted the extra safety of the extra
reference because he didn't want to trust or hadn't noticed the
extent_buffer_under_io() magic.  Auto-san, can you confirm or deny? :)