Re: please DON'T run 2.5.27 with IDE!

From: Bartlomiej Zolnierkiewicz (B.Zolnierkiewicz@elka.pw.edu.pl)
Date: Tue Jul 23 2002 - 15:24:03 EST


 Tue, 23 Jul 2002, Marcin Dalecki wrote:

> Bartlomiej Zolnierkiewicz wrote:
>
> > Martin why aren't you telling people all facts?
> > It was the default behaviour before your change in IDE 99.
> > This patch in practice reverts IDE 99 change.
> >
> > You have INTRODUCED a bug and now you try to
> > pretend that it wasn't your fault and it was somehow broken before.
>
> Never said that. Sure it was my fault I looked in the wrong direction
> I looked at the ide-tcq code, becouse I still dont like the
> idea that we pass a pointer for a struct on the local stack down.
> (It's preventing the futile hope to make this thingee somehow
> asynchronous form ever taking place.)
>
> I should have looked at SCSI in first place instead indeed.
>
> > Before 2.5.27 code had the same functionality as scsi version.
>
> That's actually not true... At least the setting of the
> request rq->flags is significantly different here and there.

You are right here...
Actually IDE request should have REQ_BARRIER bit also set for safety
and coherency.
Without barrier requests added after special command can be merged
with requests added before special command.

> However I think but I'm not sure that the fact aht we have rq->special
> != NULL here has the hidded side effect that we indeed accomplish the
> same semantics.

No.

> > And yes it will be useful to move it to block layer.
>
> Done. Just needs testing. I have at least an ZIP parport drive, which
> allows me to do some basic checks.

Test everything on your production machine + main hdd.
Will make you care more about code correctness ;-).

>
> BTW.> Having a fill up request queue trashing data transfers
> is indicating that there may be are bugs in the generic block layer too.
> If it gets pushed to boundary conditions it's apparently not very

No it wasn't "pushed to boundary conditions", you screwed it, sorry.

> robust... BTW.> I never ever will understand why
> request_fn returns void instead of an status value.

So look at ide.c for example.

Regards

--
Bartlomiej

> It could save many places where evry single driver > has to call end_request explicitely. > > > > > Regards > > -- > > Bartlomiej > > > > > >>===== drivers/ide/ide-taskfile.c 1.61 vs edited ===== > >>--- 1.61/drivers/ide/ide-taskfile.c Fri Jul 19 10:18:50 2002 > >>+++ edited/drivers/ide/ide-taskfile.c Tue Jul 23 12:12:55 2002 > >>@@ -194,22 +194,16 @@ > >> request_queue_t *q = &drive->queue; > >> struct list_head *queue_head = &q->queue_head; > >> DECLARE_COMPLETION(wait); > >>+ struct request req; > >> > >> #ifdef CONFIG_BLK_DEV_PDC4030 > >> if (ch->chipset == ide_pdc4030 && buf) > >> return -ENOSYS; /* special drive cmds not supported */ > >> #endif > >> > >>- rq = __blk_get_request(&drive->queue, READ); > >>- if (!rq) > >>- rq = __blk_get_request(&drive->queue, WRITE); > >>- > >>- /* > >>- * FIXME: Make sure there is a free slot on the list! > >>- */ > >>- > >>- BUG_ON(!rq); > >>- > >>+ memset(&req, 0, sizeof(req)); > >>+ rq = &req; > >>+ > >> rq->flags = REQ_SPECIAL; > >> rq->buffer = buf; > >> rq->special = ar; > >> > > > > > > > > >

- 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/



This archive was generated by hypermail 2b29 : Tue Jul 23 2002 - 22:00:44 EST