On Mon, 29 Nov 2010 19:48:52 -0500 Ric Wheeler <rwheeler@redhat.com> wrote:
Hi Ric,
You certainly can introduce a wait longer than the flush would take, but
the proposed code does that too.
The proposed code waits "average flush time", then submits a flush for
all threads that are waiting.
My suggestion submits a flush (Which on average takes 'average flush time')
and then submits a flush for all threads that started waiting during that
time.
So we do get two flushes rather than one, but also the flush starts
straight away, so will presumably finish sooner.
I do have the wait 'adjust itself according to the speed of the device' by
making flushes wait for one flush to complete.
I'm guessing that the situation where this is an issue is when you have a
nearly constant stream of flush requests - is that right?
In that case:
- the current code would submit lots of over-lapping flush requests,
reducing the opportunity for optimising multiple requests in the
device,
- my proposal would submit a steady sequence of flushes with minimal
gaps
- the code from Darrick would submit a steady sequence of flush requests
which would be separated by pauses of 'average flush time'.
This would increase latency, but might increase throughput too, I
don't know.
So it seems to me that the performance differences between my suggesting
and Darrick's proposal are not obvious. So some testing would be
interesting.
I was going to suggest changes to Darrick's code to demonstrate my idea, but
I think that code is actually wrong, so it wouldn't be a good base to start.
In particular, the usage of a continuation seems racy.
As soon as one thread sets EXT4_FLUSH_IDLE, another thread could call
INIT_COMPLETION, before some other threads has had a chance to wake up and
test the completion status in wait_for_completion.
The effect is that the other thread would wait for an extra time+flush which
it shouldn't have to. So it isn't a serious race, but it looks wrong.
NeilBrown
--
