Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 4 Nov 2022 10:05:42 +1100
From:   Dave Chinner <david@fromorbit.com>
To:     Gao Xiang <hsiangkao@linux.alibaba.com>
Cc:     linux-xfs@vger.kernel.org, "Darrick J. Wong" <djwong@kernel.org>,
        Dave Chinner <dchinner@redhat.com>,
        Brian Foster <bfoster@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Zirong Lang <zlang@redhat.com>
Subject: Re: [PATCH v2] xfs: extend the freelist before available space check
Message-ID: <20221103230542.GK3600936@dread.disaster.area>
References: <20221103094639.39984-1-hsiangkao@linux.alibaba.com>
 <20221103131025.40064-1-hsiangkao@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221103131025.40064-1-hsiangkao@linux.alibaba.com>
Precedence: bulk

On Thu, Nov 03, 2022 at 09:10:25PM +0800, Gao Xiang wrote:
> There is a long standing issue which could cause fs shutdown due to
> inode extent to btree conversion failure right after an extent
> allocation in the same AG, which is absolutely unexpected due to the
> proper minleft reservation in the previous allocation.  Brian once
> addressed one of the root cause [1], however, such symptom can still
> occur after the commit is merged as reported [2], and our cloud
> environment is also suffering from this issue.
> 
> From the description of the commit [1], I found that Zirong has an
> in-house stress test reproducer for this issue, therefore I asked him
> to reproduce again and he confirmed that such issue can still be
> reproducable on RHEL 9.
> 
> Thanks to him, after dumping the transaction log items, I think
> the root cause is as below:
>  1. Allocate space with the following condition:
>     freeblks: 18304 pagf_flcount: 6
>     reservation: 18276 need (min_free): 6
>     args->minleft: 1
>     available = freeblks + agflcount - reservation - need - minleft
>               = 18304 + min(6, 6) - 18276 - 6 - 1 = 27
> 
>     The first allocation check itself is ok;
> 
>  2. At that time, the AG state is
>     AGF Buffer: (XAGF)
>         ver:1  seq#:3  len:2621424
>         root BNO:9  CNT:7
>         level BNO:2  CNT:2
>         1st:64  last:69  cnt:6  freeblks:18277  longest:6395
                                  ^^^^^^^^^^^^^^

Hold on - pag->pagf_freeblks != agf->freeblks, and if we start with
the agf freeblocks:

	available = 18277 + 6 - 18276 - 6 - 1 = 0

IOWs, the allocation should never selected this AG in the first
place.

So why is pag->pagf_freeblks not equal to agf->freeblks when this
allocation was first checked? It's clearly not because the AGFL is
unpopulated - both the perag and the agf indicate it has the minimum
6 blocks already allocated....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com