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

[David Sterba <dsterba@suse.cz>]

On Wed, May 31, 2023 at 03:04:26PM +0000, Naohiro Aota wrote:
> On Wed, May 31, 2023 at 06:16:26AM +0200, Christoph Hellwig wrote:
> > On Tue, May 30, 2023 at 05:56:48PM +0200, David Sterba wrote:
> > > > > 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? :)
> > > 
> > > The number of patches above this one in the queue is increasing so it
> > > would get harder to remove it. I took another look and agree that
> > > regarding the references it's safe but would still like a confirmation.
> > 
> > As stated, I am very confident that this is safe based on all my
> > recent work with the extent_buffer code base.  I'd love to hear
> > from Aota, but there's not much more I can add here myself.
> 
> Sorry. I missed this thread is on-going.
> 
> I ran my test runs on misc-next containing this patch, and got no issue
> regarding this. So, the patch should be good.
> 
> I didn't notice the extent_buffer_under_io() magic. If we can remove it,
> let's remove unnecessary variable from extent_buffer.
> 
> Also, I dig into the "redirty" history to make it sure. In the first place,
> it used releasing_list to hold all the to-be-released extent buffers, and
> decided which buffers to re-dirty at the commit time. Then, in a later
> version, I change the behavior to re-dirty a necessary buffer and add
> re-dirtied one to the list in btrfs_free_tree_block(). In short, the list
> was there mostly for the patch series' historical reason.
> 
> So, not sure still I can add this but, for the whole series:
> 
> Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>

Perfect, thanks. Changelog updated and rev-by added.