On Mon, Jan 1, 2018 at 11:02 PM, <tang.junhui@xxxxxxxxxx> wrote: > From: Tang Junhui <tang.junhui@xxxxxxxxxx> > > I noticed you add "closure_sync(&cl)" before assigning delay to zero in your patch, > I think we should add it before: > delay = writeback_delay(dc, size) > otherwise we would alays get a wrong value of delay after calling writeback_delay(), > since the write-back IO is just sended out and not finished yet. > > Am I right? I don't think so. The thing that is controlled (in current code, and this patch set) is the rate of issuance, not of completion (though issuance rate is guaranteed not to exceed completion rate, because of the semaphore for the maximum number of I/Os outstanding). The intention with this code is to notice when we are ahead on schedule on writeback, and if so, start the next I/O after all of the current set finishes. Only in this case do we do closure_sync, as a way to wait for all the existing I/Os to complete. This guarantees that we never have more than one I/O outstanding when doing "opportunistic idle writeback", and that new front-end I/O never has to compete with more than one thing in the queue in this case. Mike
