Re: [PATCH v2] xfs: extend the freelist before available space check

[Gao Xiang]

On Fri, Nov 04, 2022 at 08:31:04AM +0800, Gao Xiang wrote:
> Hi Dave,
> 
> On Fri, Nov 04, 2022 at 10:05:42AM +1100, Dave Chinner wrote:
> > 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....
> 
> Thanks for the reply.
> 
> I may mispresent 2) here since there are several AGF agno 3 recording,
> the last completed trans printed by "xfs_logprint" is:
> 
> ============================================================================
> TRANS: tid:0xaf57a744  #items:621  trans:0xaf57a744  q:0x56104c44be70
> CUD: cnt:1 total:1 a:0x56104c44e320 len:16
> CUD:  #regs: 1                   id: 0xff110004e02bc1e8
> EFI: cnt:1 total:1 a:0x56104c447b30 len:32
>         EFI:  #regs:1   num_extents:1  id:0xff110001bd8c56e0
>         (s: 0xe7cc8d, l: 3)
> EFD: cnt:1 total:1 a:0x56104c42d1b0 len:32
>         EFD:  #regs: 1  num_extents: 1  id: 0xff110001bd8c56e0
> BUF: cnt:2 total:2 a:0x56104c42f5c0 len:24 a:0x56104c4712e0 len:128
>         BUF:  #regs:2   start blkno:0x3bffe81   len:1   bmap size:1   flags:0x2800
>         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:18304  longest:6395
> 
> So I think freeblks starts from 18304.
> 
> 18277 is just an intermediate state in my mind (Actually there is also such AGF
> record, but that is not the latest one because this is a stress test), sorry
> for this.
> 
> In short, in order to do the first allocation, I think it allocates from
>  freeblks 18304 -> 18276


                      ^ sorry 18277 here, already too many numbers in my head

I tried to send dmesg.log.xz in this thread, since I'm not sure whether
@vger.kernel.org will drop this email directly or not.

Thanks,
Gao Xiang


>  agflcount 6->5
> And the second one fails,
>      available = freeblks + agflcount - reservation - need - minleft
>                = 18277 + min(5, 6) - 18276 - 6 - 0 = 0   < 1
> I also think it can happen in the current codebase.
> 
> Full xfs_logprint is too large to send by email to the mailing list, but
> I could send this separately to you if really needed.
> 
> My debugging message catched when xfs_trans_cancel() attached in the
> following reply of this email.
> 
> Thanks,
> Gao Xiang
> 
> > 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@xxxxxxxxxxxxx

Attachment: dmesg.log.xz
Description: Binary data