On Thu, Sep 19, 2024 at 09:46:31AM +0200, Christoph Hellwig wrote: > On Thu, Sep 19, 2024 at 11:09:40AM +1000, Dave Chinner wrote: > > Ideally, we should not be using the new AGs until *after* the growfs > > transaction has hit stable storage (i.e. the journal has fully > > commmitted the growfs transaction), not just committed to the CIL. > > Yes. A crude version of that - freeze/unfreeze before setting the > AG live was my other initial idea, but Darrick wasn't exactly > excited about that.. I'm not exactly excited by that idea, either... > > The second step is preventing allocations that are running from > > seeing the mp->m_sb.sb_agcount update until after the transaction is > > stable. > > Or just not seeing the pag as active by not setting the initial > active reference until after the transaction is stable. Given > all the issues outlined by you about sb locking that might be the > easier approach. Yeah, that's a good idea for avoiding perag references from iterations before the growfs is stable. However, my concern is whether that is sufficient. Whilst I didn't mention it, changing sb_agcount and sb_dblocks before the grwofs is stable affects things like size calculations for the old end runt AG(*) because it is no longer considered a runt the moment we change the in-memory size fields in the superblock. That will, at least, affect ino/fsbno/agbno verification, as well as corruption checks through the code. An alternative is to delay the perag initialisation until after the growfs is stable, because we don't want the old runt AG size to visibly change until after the growfs is stable. There may be more potential issues, but I haven't done a careful code audit and that's kinda why I suggested simply delaying the in-memory state update. Delaying the update removes the whole in-memory transient state situation altogether... -Dave. (*) The precalculated AG length and inode min/max values we've added to the perag (calculated at perag init time) should be used for these calculations. That gets around this 'growfs changes sb values dynamically' issue, but not all the places where we do these verifications have a valid perag reference to pass these functions. -- Dave Chinner david@xxxxxxxxxxxxx
