On Mon, Jan 1, 2018 at 11:02 PM,  [off-list ref] wrote:

From: Tang Junhui <redacted>

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