On Thu, Sep 23, 2010 at 01:53:55PM -0500, Alex Elder wrote: > On Thu, 2010-09-23 at 12:27 +1000, Dave Chinner wrote: > > + * than half the log" rule that recovery requires us to keep. > > + * > > + * Further, we need to make sure the background CIL push is efficient, which > > + * means we need to give the background push a chance to commit without > > + * blocking all the current transaction commits. Hence we need some space > > + * between the threshold and the 25% limit to allow background pushes to be > > + * tried, but not enforced. To make this simple and fast to calculate, set > > + * the background push threshold to 1/8th (12.5%) the size of the log, and then start > > + * enforcing the background push at 50% above this. i.e. at 3/16th or 18.75% of > > + * the log size. This should keep us well under the limits of the AIL pushing > > + * threshold, yet give us plenty of space for aggregation on large logs. > > */ > > I think the above explanation is pretty good but I don't know that it's > as clear or concise as it could be. I don't claim this is better but > I'll take a shot (I don't like offering criticism without suggesting > an alternative). > > * With dynamic reservations, we can basically make up arbitrary > * limits for the checkpoint size so long as they don't violate any > * other size rules. Recovery imposes a rule that no transaction > * exceed half the log, so we are limited by that. Furthermore, the > * log transaction reservation subsystem tries to keep 25% of the > * log free, so we should keep below that limit or we risk not being > * able to get the space we need. > * > * In order to keep background CIL push efficient, we will set a > * lower threshold at which background pushing is attempted without > * blocking current transaction commits. A separate, higher bound > * defines when CIL pushes are forced in order to ensure we stay > * within our transaction size limits. Yes, makes sense. I'll rework it along these lines. > > - > > -#define XLOG_CIL_SPACE_LIMIT(log) \ > > - (min((log->l_logsize >> 2), (8 * 1024 * 1024))) > > +#define XLOG_CIL_SPACE_LIMIT(log) (log->l_logsize >> 3) > > +#define XLOG_CIL_HARD_SPACE_LIMIT(log) (3 * (log->l_logsize >> 4)) > > Maybe "LIMIT" isn't quite the right name for these two. > (But I have no better suggestion.) Threshold is really the only other word that matches, but I think limit is better here as it conveys a sense that it is something we don't really want to cross... > I don't really care much about this, but I'll take this > opportunity for a small rant. > > The difference in calculation cost/speed between "x >> 3" and > "x / 8" is vanishingly small. That is architecture dependent, but in most cases these days the compiler will optimise a divide-by-power-ofâ2-constant into a shift operation anyway. I'm pretty sure that optimisation is done on even on x86 as a shift is a single cycle operation while an integer divide still takes several cycles and consumes more power. > I think it is meaningful to use > a shift in places where a power-of-two is mandated, but in places > like this it suggests there is a constraint that simply doesn't > exist. So for example, you could have chosen (log->logsize / 10) > as the "try pushing" value, and (log->logsize / 4 - 1) as the > "must push" value. It's more the fact that XFS uses power-of-2 logic (i.e shifts) everywhere. I just tend to be consistent with what is already there. In this case, the AIL push thresholds are calculated using shifts: free_threshold = MAX(free_threshold, (log->l_logBBsize >> 2)); and so when you compare that to the XLOG_CIL_SPACE_LIMIT() definitions, it is immediately clear that the CIL limits are smaller than the AIL push threshold... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
