Re: [PATCH linux-2.6-block:post-2.6.15 08/10] blk: update IDE to use new blk_ordered

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jens Axboe wrote:
On Wed, Nov 23 2005, Bartlomiej Zolnierkiewicz wrote:

On 11/23/05, Tejun Heo <[email protected]> wrote:

On Tue, Nov 22, 2005 at 09:36:09AM +0100, Bartlomiej Zolnierkiewicz wrote:
[--snip--]

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.

ditto


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
description).


Yeap, both suck.  Glad to hear that.  :-)


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.


Hi, Bartlomiej.  Hi, Jens.

Currently, there are only two more things to do before getting this thing into the -mm tree.

1. Adjusting this patch (update-ide) after merging Bartlomiej's del_gendisk() fix patch.

2. Update ide-fua patch as discussed and get it reviewed by Bartlomiej.

Other than above two, I think we're ready. I'll post update ide-fua patch later today.

Thanks.

--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux