Messages in this thread | | | Date | Tue, 29 May 2001 02:30:29 +0200 | From | Jens Axboe <> | Subject | Re: [patch]: ide dma timeout retry in pio |
| |
On Mon, May 28 2001, Andre Hedrick wrote: > On Tue, 29 May 2001, Jens Axboe wrote: > > > This is bull shit. If IDE didn't muck around with the request so much in > > the first place, the info could always be trusted. Even so, we have the > > hard_* numbers to go by. So this argument does not hold. > > Maybe if you looked at the new code model as a whole you would see that > the request-forking is gone. The object is to preserve a copy of the io
There's only a 'fork' (I'm assuming you mean copy?) for the pio multwrite path, which I don't consider a big issue. I'm not saying that TFAM is not needed, I'm saying don't go touting that as the big saviour when it really has no relation to this topic at all.
> instructions out to the registers to not have to repeat the do_request > call unless it is a do or die thing. Also it is good to carry a copy of > the local request even if it is never used. Also are you resetting the
Retry is by no means a performance thing, and I would indeed prefer to just pretend this is a brand new request rather than count on anything being in a sane state.
> pointer (back to the geginning) on rq->buffer on the retry?
Of course
> You first flush the DMA engine and issue a device soft reset not using the > current drive reset, is presevers the hwgroup->busy state and allows the > request to be retried without reinserting.
Again, _there is no reinsertion_. And why would we want to preserve hwgroup->busy? In fact, we need to clear it at all times to start the request over sanely.
> > > As I recall, there is a way to reinsert the faulted request, but that > > > > Again, look at the patch. The request is never off the list, so there is > > never a reason to reinsert. hwgroup->busy is cleared (and, again for > > good measure, hwgroup->rq), so ide_do_request/start_request will get the > > same request that we just handled. > > I will have to poke in a few flags to verify this but if you say so.
Ok, this is how it goes: a queue list is build of requests. IDE grabs the very first request on the list, the path:
ide_do_request: hwgroup->rq = blkdev_entry_next_request(&drive->queue.queue_head)
start_request: struct request *rq = blkdev_entry_next_request(&drive->queue.queue_head)
There is no deletion going on. Request is started, runs, and is done. At this point the low level driver calls ide_end_request:
ide_end_request: if (last_part_of_request) { blkdev_dequeue_request(rq); hwgroup->rq = NULL; ... }
_here_ we take the request off the list, and we better be completely done with it at this point. So as long as the request is not ended completely, it will always be the at the head of the pending queue.
> > > means the request_struct needs fault counter. If it is truly a DMA error > > > > ->errors, it's already there. > > Wrong location to poke and by that time it requires a full retry. > The new code would have had the task structs filled with the error.
->errors is just meant as a simple counter, so you can do stuff like
if (io_error) if (rq->errors++ > ERROR_MAX) /* uh oh */
if you need more than this, then yes a dedicated 'ide command' pointer is what we want. You would need that for queueing anyway.
-- Jens Axboe
- 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/
| |