lkml.org 
[lkml]   [2005]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH linux-2.6-block:post-2.6.15 08/10] blk: update IDE to use new blk_ordered
Hello, Bartlomiej.

Bartlomiej Zolnierkiewicz wrote:
> On 11/17/05, Tejun Heo <htejun@gmail.com> wrote:
>
> I fail to see how the partial completions (good + bad sectors)
> are done in your new scheme, please explain.
>

It doesn't. I've noted this way back when I posted this patchset the
second time.

http://marc.theaimsgroup.com/?l=linux-kernel&m=111795127124020&w=2

Rationales

* The actual barrier IO request is issued as a part of ordered sequence.
When any part of this sequence fails (any of leading flush, barrier IO
or post flush), the whole sequence should be considered to have failed.

e.g. if leading flush fails, there's no point in reporting partial or
full success of barrier IO. Ditto for tailing flush. We can special
case when only part of barrier IO fails and report partial barrier
success, but 1. benefits are doubtful 2. even if it's implemented, it
wouldn't work (see next rationale)

* Barrier requests are not mergeable. ie. Each barrier bio is turned
into one barrier request and partially completing the request doesn't
result in any successfully completed bio.

* SCSI doesn't handle partial completion of barrier IOs.

>
>>-
>>-static int idedisk_prepare_flush(request_queue_t *q, struct request *rq)
>>-{
>>- ide_drive_t *drive = q->queuedata;
>>-
>>- if (!drive->wcache)
>>- return 0;
>
>
> What does happen if somebody disables drive->wcache later?
>

Thanks for pointing out. I've moved ordered configuration into
write_cache such that ordered is reconfigured when write_cache changes.

There can be in-flight barrier requests which are inconsistent with the
newly updated setting, but 1. it's not too unfair to assume that user is
responsible for that synchronization 2. the original implementation had
the same issue 3. the consequence is not catastrophic.

>
>> memset(rq->cmd, 0, sizeof(rq->cmd));
>>
>>@@ -735,9 +694,8 @@ static int idedisk_prepare_flush(request
>> rq->cmd[0] = WIN_FLUSH_CACHE;
>>
>>
>>- rq->flags |= REQ_DRIVE_TASK | REQ_SOFTBARRIER;
>>+ rq->flags |= REQ_DRIVE_TASK;
>> rq->buffer = rq->cmd;
>>- return 1;
>> }
>>
>> static int idedisk_issue_flush(request_queue_t *q, struct gendisk *disk,
>>@@ -1012,11 +970,12 @@ static void idedisk_setup (ide_drive_t *
>> printk(KERN_INFO "%s: cache flushes %ssupported\n",
>> drive->name, barrier ? "" : "not ");
>> if (barrier) {
>>- blk_queue_ordered(drive->queue, QUEUE_ORDERED_FLUSH);
>>- drive->queue->prepare_flush_fn = idedisk_prepare_flush;
>>- drive->queue->end_flush_fn = idedisk_end_flush;
>>+ blk_queue_ordered(drive->queue, QUEUE_ORDERED_DRAIN_FLUSH,
>>+ idedisk_prepare_flush, GFP_KERNEL);
>> blk_queue_issue_flush_fn(drive->queue, idedisk_issue_flush);
>>- }
>>+ } else if (!drive->wcache)
>>+ blk_queue_ordered(drive->queue, QUEUE_ORDERED_DRAIN,
>>+ NULL, GFP_KERNEL);
>
>
> What does happen if somebody enables drive->wcache later?
>

ditto.

>
>> }
>>
>> static void ide_cacheflush_p(ide_drive_t *drive)
>>@@ -1034,6 +993,8 @@ static int ide_disk_remove(struct device
>> struct ide_disk_obj *idkp = drive->driver_data;
>> struct gendisk *g = idkp->disk;
>>
>>+ blk_queue_ordered(drive->queue, QUEUE_ORDERED_NONE, NULL, 0);
>>+
>
>
> Shouldn't this be done in ide_disk_release()?

Hmmm... The thing is that, AFAIK, requests are not supposed to be issued
after ->remove is called (->remove is called only on module unload
unless hardware is hot-unplugged and HL driver cannot be unloaded while
it's still opened). I think that's why both sd and ide-disk issue the
last cache flush in ->remove callbacks but not in ->release.

I think the most natural place to put ordered deconfiguration is right
above the last cache flush. Hmmm... If above is not true, I think we
should move both ordered deconfig and the last cache flushes to
->release callbacks. What do you think?

Thanks.

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

\
 
 \ /
  Last update: 2005-11-18 16:10    [W:0.174 / U:0.324 seconds]
©2003-2014 Jasper Spaans. Advertise on this site