Re: [PATCH 5/8] btrfs: zoned: activate metadata block group on write time

["hch@infradead.org" <hch@infradead.org>]

On Tue, Jul 25, 2023 at 09:28:09AM +0000, Naohiro Aota wrote:
> > > +		bool is_system = cache->flags & BTRFS_BLOCK_GROUP_SYSTEM;
> > > +
> > > +		spin_lock(&cache->lock);
> > > +		if (test_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE,
> > > +			     &cache->runtime_flags)) {
> > > +			spin_unlock(&cache->lock);
> > > +			return true;
> > > +		}
> > > +
> > > +		spin_unlock(&cache->lock);
> > 
> > What is the point of the cache->lock crticial section here?  The
> > information can be out of date as soon as you drop the lock, so it
> > looks superflous to me.
> 
> The block group's activeness starts from non-active, then activated, and
> finally finished. So, if its "ACTIVE" here, the block group is going to be
> still active or finished. It's OK it is active. If it is finished, the IO
> will fail anyway, so it is a bug itself.

Yes, but what I mean is that there is no point in taking cache->lock
here.  The only thing that's done under it is testing a single bit,
and the information from that bit is only used after dropping the
lock.  So there should be no need to take the lock in the first place.