> > Hi Brian, > > > > This patch was one of my selected fixes to backport for v5.10.y. > > It has a very scary looking commit message and the change seems > > to be independent of any infrastructure changes(?). > > > > The problem is that applying this patch to v5.10.y reliably reproduces > > this buffer corruption assertion [*] with test xfs/076. > > > > This happens on the kdevops system that is using loop devices over > > sparse files inside qemu images. It does not reproduce on my small > > VM at home. > > > > Normally, I would just drop this patch from the stable candidates queue > > and move on, but I thought you might be interested to investigate this > > reliable reproducer, because maybe this system exercises an error > > that is otherwise rare to hit. > > > > It seemed weird to me that NOT reusing the extent would result in > > data corruption, but it could indicate that reusing the extent was masking > > the assertion and hiding another bug(?). > > > > Indeed, this does seem like an odd failure. The shutdown on transaction > cancel implies cancellation of a dirty transaction. This is not > necessarily corruption as much as just being the generic > naming/messaging related to shutdowns due to unexpected in-core state. > The patch in question removes some modifications to in-core busy extent > state during extent allocation that are fundamentally unsafe in > combination with how allocation works. This change doesn't appear to > affect any transaction directly, so the correlation may be indirect. > > xfs/076 looks like it's a sparse inode allocation test, which certainly > seems relevant in that it is stressing the ability to allocate inode > chunks under free space fragmentation. If this patch further restricts > extent allocation by removing availability of some set of (recently > freed, busy) extents, then perhaps there is some allocation failure > sequence that was previously unlikely enough to mask some poor error > handling logic or transaction handling (like an agfl fixup dirtying a > transaction followed by an allocation failure, for example) that we're > now running into. > > > Can you think of another reason to explain the regression this fix > > introduces to 5.10.y? > > > > Not off the top of my head. Something along the lines of the above seems > plausible, but that's just speculation at this point. > > > Do you care to investigate this failure or shall I just move on? > > > > I think it would be good to understand whether there's a regression > introduced by this patch, a bug somewhere else or just some impedence > mismatch in logic between the combination of this change and whatever > else happens to be in v5.10.y. Unfortunately I'm not able to reproduce > if I pull just this commit back into latest 5.10.y (5.10.118). I've > tried with a traditional bdev as well as a preallocated and sparse > loopback scratch dev. I also failed to reproduce it on another VM, but it reproduces reliably on this system. That's why I thought we'd better use this opportunity. This system has lots of RAM and disk to spare so I have no problem running this test in a VM in parallel to my work. It is not actually my system, it's a system that Luis has setup for stable XFS testing and gave me access to, so if the need arises you could get direct access to the system, but for now, I have no problem running the test for you. > Have you tested this patch (backport) in isolation > in your reproducer env or only in combination with other pending > backports? > I tested it on top of 5.10.109 + these 5 patches: https://github.com/amir73il/linux/commits/xfs-5.10.y-1 I can test it in isolation if you like. Let me know if there are other forensics that you would like me to collect. Thanks, Amir.
