On Fri, Feb 28, 2014 at 07:24:05AM +1100, Dave Chinner wrote: > On Fri, Feb 28, 2014 at 07:01:50AM +1100, Dave Chinner wrote: > > On Thu, Feb 27, 2014 at 09:08:46AM -0500, Brian Foster wrote: > > > On Thu, Feb 27, 2014 at 08:51:14PM +1100, Dave Chinner wrote: > > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > > > > > Hmm, I replied to this one in the previous thread, but now I notice that > > > it apparently never made it to the list. Dave, did you happen to see > > > that in your inbox? Anyways, I had a couple minor comments/questions > > > that I'll duplicate here (which probably don't require another > > > repost)... > > > > No, I didn't. > > > > [snip typos that need fixing] > > > > > > diff --git a/repair/prefetch.c b/repair/prefetch.c > > > > index aee6342..7d3efde 100644 > > > > --- a/repair/prefetch.c > > > > +++ b/repair/prefetch.c > > > > @@ -866,6 +866,48 @@ start_inode_prefetch( > > > > return args; > > > > } > > > > > > > > > > A brief comment before the prefetch_ag_range bits that explain the > > > implicit design constraints (e.g., throttle prefetch based on > > > processing) would be nice. :) > > > > Can do. > > Added this: > > /* > * prefetch_ag_range runs a prefetch-and-process loop across a range of AGs. It > * begins with @start+ag, and finishes with @end_ag - 1 (i.e. does not prefetch > * or process @end_ag). The function starts prefetch on the first AG, then loops > * starting prefetch on the next AG and then blocks processing the current AG as > * the prefetch queue brings inodes into the processing queue. > * > * There is only one prefetch taking place at a time, so the prefetch on the > * next AG only starts once the current AG has been completely prefetched. Hence > * the prefetch of the next AG will start some time before the processing of the > * current AG finishes, ensuring that when we iterate an start processing the and > * next AG there is already a significant queue of inodes to process. > * > * Prefetch is done this way to prevent it from running too far ahead of the > * processing. Allowing it to do so can cause cache thrashing, where new > * prefetch causes previously prefetched buffers to be reclaimed before the > * processing thread uses them. This results in reading all the inodes and > * metadata twice per phase and it greatly slows down the processing. Hence we > * have to carefully control how far ahead we prefetch... > */ > Looks good, thanks! Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx> > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
