On Wed, Nov 23 2005, Bartlomiej Zolnierkiewicz wrote:
On 11/23/05, Tejun Heo <htejun@xxxxxxxxx> wrote:
On Tue, Nov 22, 2005 at 09:36:09AM +0100, Bartlomiej Zolnierkiewicz wrote:
Ordered requests are processed in the following order.
1. barrier bio reaches blk queue
2. barrier req queued in order
3. when barrier req reaches the head of the request queue, it gets
interpreted into preflush-barrier-postflush requests sequence
and queued. ->prepare_flush_fn is called in this step.
4. When all three requests complete, the ordered sequence ends.
Adding !drive->wcache test to idedisk_prepare_flush, which in turn
requires adding ->prepare_flush_fn error handling to blk ordered
handling, prevents flushes for barrier requests between step#1 and
Why for !drive->wcache flush can't be consider as successful
like it was before these changes...
step#3. We can still have flush reqeuests between #3 and #4 after
wcache is turned off.
I think we have two alternatives here - both have some problems.
1. make ->prepare_flush_fn return some code to tell blk layer skip
the flush as the original code did.
This is what you're proposing, I guess. The reason why I'm reluctant
to take this approach is that there still remains window of error
between #3 and #4. The flush requests could already be prepared and
in the queue when ->wcache is turned off. AFAICS, the original code
had the same problem, although the window was narrower.
2. complete flush commands successfully in execute_drive_cmd() if wcache
is not enabled.
This approach fixes all cases but the implementation would be a bit
hackish. execute_drive_cmd() will have to look at the command and
ide_disk->wcache to determine if a special command should be completed
successfully without actually executing it. Maybe we can add a
per-HL-driver callback for that.
Bartlomiej, I'm not really sure about both of above approaches. My
humble opinion is that benefits brought by both of above aren't worth
the added complexity. The worst can happen is a few IDE command
failures caused by executing flush command on a wcache disabled drive.
And that would happen only when the user turns off wcache while
barrier sequence is *in progress*.
Hmmm... What do you think? It's your call. I'll do what you choose.
Hmm... both solutions sucks. After second thought I agree with you
w.r.t to original changes (just remember to document them in the patch
Me too. Plus on most drives a flush cache command on a drive with write
back caching disabled will succeed, not fail. t13 is about to mandate
that behaviour as well, and honestly I think this is the logical way to
implement it in a drive (the flush just becomes a noop).
So Tejun, where do we stand with this patch series? Any changes over
what you posted last week? I'd like to get this merged in the block
tree, get it in -mm and merged for 2.6.16 if things work out.