On Wed, Dec 14, 2016 at 12:35:07PM -0500, Brian Foster wrote: > I'm still trying to grok the minleft/dop_low stuff, but doesn't this > increase the risk of bmbt block allocation failure? No. If working correctly it reduces the chance of a failing bmbt block allocation in transaction to zero, at the expense of potentially failing whole block allocations while we still have a few blocks left. But given that the first leads to a fs shutdown and the latter just to an ENOSPC it seems worthwhile. > E.g., args.minleft > is set to the transaction reservation earlier in the function. AFAIK the > res is not guaranteed to be in a particular AG, but in that case we're > just setting a start AG and ultimately cycling through the AGs. If that > fails, this hunk resets minleft, restarts at fsbno 0 and activates the > sequential agno allocation requirement for subsequent allocs. Doesn't > removing this logic mean we can successfully reserve blocks in a write > transaction that will never ultimately be able to allocate bmbt blocks, > even if the data blocks can all be allocated..? We only set it to res if no firstblock was set. The only case where that is possible during the first bmbt block allocation in a reflink operation - for actual direct I/O writes, fallocate calls or unwritten extent conversion we will always have firstblock set from the allocation of the actual data extent. And that allocation now has the proper minleft set after this series so that we are guaranteed we have enough space in one single AG for the whole allocation. In the reflink case where firstblock wasn't set the minleft ensures that we fail the first allocation if we don't have enough blocks in a single AG. retryloop to try the allocation > The other thing to note here is that minleft is initialized to 0 and > only set to the transaction res conditionally, so if firstblock is > already set we won't use minleft at all. Yes, that's intentional - we already did our minleft check when allocating the data extent. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html