Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id ; Mon, 30 Sep 2002 07:49:51 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id ; Mon, 30 Sep 2002 07:49:51 -0400 Received: from ns.virtualhost.dk ([195.184.98.160]:15803 "EHLO virtualhost.dk") by vger.kernel.org with ESMTP id ; Mon, 30 Sep 2002 07:49:46 -0400 Date: Mon, 30 Sep 2002 13:54:21 +0200 From: Jens Axboe To: Andre Hedrick Cc: Alan Cox , Linus Torvalds , james , Ingo Molnar , Jeff Garzik , Larry Kessler , linux-kernel mailing list , "Andrew V. Savochkin" , Rusty Russell , Richard J Moore Subject: Re: v2.6 vs v3.0 Message-ID: <20020930115421.GA979@suse.de> References: <20020930075625.GH1014@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8987 Lines: 267 On Mon, Sep 30 2002, Andre Hedrick wrote: > On Mon, 30 Sep 2002, Jens Axboe wrote: > > > On Sun, Sep 29 2002, Alan Cox wrote: > > > On Sun, 2002-09-29 at 18:42, Linus Torvalds wrote: > > > > I can say that the IDE code is the same code that is in 2.4.x, so if > > > > you're comfortable with 2.4.x wrt IDE, then you should be comfy with > > > > 2.5.x too. > > > > > > *NO* > > > > > > The IDE code is the experimental code in 2.4-ac. It is _NOT_ the IDE > > > code in 2.4 and its a lot less tested. I don't think it has any > > > corruption bugs but it is most definitely not the base 2.4 code and has > > > plenty of non corruption bugs (PCMCIA hang, taskfile write hang, irq > > > blocking performance problems) > > > > 2.5 at least does not have the taskfile hang, because I killed taskfile > > io. > > Great :-/ Now that you have restored the "rq->wrq" aka working copy of Make taskfile io work 2.4-ac, and it will work in 2.5 as well. The only sensible thing to do right now was to disable it in 2.5, imo, and so I did. > the request which in its past life under PIO only updated to block when > the entire request was completed. So there are no partial completions > possible given the old method in the legacy path. I haven't restored anything. 2.4-ac (your base) uses ->wrq copy, so does 2.5. > One of the issues Linus kick my can over was the "requirement" of partial > completeions. What I need rom block is a way to know how much is > completed of the original total request. So whatever value is the > original rq->nr_sectors assigned to "TF.2/HF.2" or nsector_offset(s), > needs to be carried in block and updated to reflect how much more is > remaining of this CDB task. Now that the block layer really can do partial completions properly, I patched ide-disk to do just that. It's not very well tested, just did it last week as proof-of-concept. This breaks the typical offset rules, ie current_segment_offset = rq->hard_cur_sectors - rq->current_nr_sectors; total_offset = rq->hard_nr_sectors - rq->nr_sectors; Haven't though too much about that yet. > I do not care if you call it "rq->dumbass_accounting_for_andre", but > provide this dummy accounting variable in "struct request" and I will be > happy. This has nothing to do with bio or bh segments from the kernel. > It is everything about device side accounting carried by block; whereas, > the ll_driver can use it to determine what or if there is to be another > interrupt. What you ask for is already there, but requires that you massage current_nr_sectors and nr_sectors like ide has always done. > Why are we getting lost interrupts? > > Because there is a beautiful "data-block completion" v/s "immediate > interrupt assertion" race between the device and the kernel. So please > provide a counter which can be used to determine where the interrupt > driven partial completion model the driver is wrt the device/request. > > Jens, not asking for much. Indeed, you are asking for stuff we've had for years. ===== drivers/ide/ide-disk.c 1.16 vs edited ===== --- 1.16/drivers/ide/ide-disk.c Sat Sep 21 02:32:22 2002 +++ edited/drivers/ide/ide-disk.c Mon Sep 23 17:18:48 2002 @@ -139,8 +139,8 @@ */ static ide_startstop_t read_intr (ide_drive_t *drive) { - ide_hwif_t *hwif = HWIF(drive); - int i = 0, nsect = 0, msect = drive->mult_count; + ide_hwif_t *hwif = HWIF(drive); + int nsect = 0, msect = drive->mult_count; struct request *rq; unsigned long flags; u8 stat; @@ -174,25 +174,24 @@ (unsigned long) rq->buffer+(nsect<<9), rq->nr_sectors-nsect); #endif ide_unmap_buffer(rq, to, &flags); - rq->sector += nsect; - rq->errors = 0; - i = (rq->nr_sectors -= nsect); - if (((long)(rq->current_nr_sectors -= nsect)) <= 0) - ide_end_request(drive, 1, rq->hard_cur_sectors); + + /* + * all done + */ + if (!ide_end_request(drive, 1, nsect)) + return ide_stopped; + /* * Another BH Page walker and DATA INTERGRITY Questioned on ERROR. * If passed back up on multimode read, BAD DATA could be ACKED * to FILE SYSTEMS above ... */ - if (i > 0) { - if (msect) - goto read_next; - if (HWGROUP(drive)->handler != NULL) - BUG(); - ide_set_handler(drive, &read_intr, WAIT_CMD, NULL); - return ide_started; - } - return ide_stopped; + if (msect) + goto read_next; + if (HWGROUP(drive)->handler != NULL) + BUG(); + ide_set_handler(drive, &read_intr, WAIT_CMD, NULL); + return ide_started; } /* @@ -203,7 +202,6 @@ ide_hwgroup_t *hwgroup = HWGROUP(drive); ide_hwif_t *hwif = HWIF(drive); struct request *rq = hwgroup->rq; - int i = 0; u8 stat; if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG), @@ -217,23 +215,19 @@ rq->nr_sectors-1); #endif if ((rq->nr_sectors == 1) ^ ((stat & DRQ_STAT) != 0)) { - rq->sector++; - rq->errors = 0; - i = --rq->nr_sectors; - --rq->current_nr_sectors; - if (((long)rq->current_nr_sectors) <= 0) - ide_end_request(drive, 1, rq->hard_cur_sectors); - if (i > 0) { - unsigned long flags; - char *to = ide_map_buffer(rq, &flags); - taskfile_output_data(drive, to, SECTOR_WORDS); - ide_unmap_buffer(rq, to, &flags); - if (HWGROUP(drive)->handler != NULL) - BUG(); - ide_set_handler(drive, &write_intr, WAIT_CMD, NULL); - return ide_started; - } - return ide_stopped; + unsigned long flags; + char *to; + + if (!ide_end_request(drive, 1, 1)) + return ide_stopped; + + to = ide_map_buffer(rq, &flags); + taskfile_output_data(drive, to, SECTOR_WORDS); + ide_unmap_buffer(rq, to, &flags); + if (HWGROUP(drive)->handler != NULL) + BUG(); + ide_set_handler(drive, &write_intr, WAIT_CMD, NULL); + return ide_started; } /* the original code did this here (?) */ return ide_stopped; ===== drivers/ide/ide-taskfile.c 1.4 vs edited ===== --- 1.4/drivers/ide/ide-taskfile.c Fri Sep 20 00:13:51 2002 +++ edited/drivers/ide/ide-taskfile.c Mon Sep 23 17:04:47 2002 @@ -611,9 +611,8 @@ * BH walking or segment can only be updated after we have a good * hwif->INB(IDE_STATUS_REG); return. */ - if (--rq->current_nr_sectors <= 0) - if (!DRIVER(drive)->end_request(drive, 1, 0)) - return ide_stopped; + if (!DRIVER(drive)->end_request(drive, 1, 1)) + return ide_stopped; /* * ERM, it is techincally legal to leave/exit here but it makes * a mess of the code ... @@ -669,7 +668,6 @@ taskfile_input_data(drive, pBuf, nsect * SECTOR_WORDS); task_unmap_rq(rq, pBuf, &flags); rq->errors = 0; - rq->current_nr_sectors -= nsect; msect -= nsect; /* * FIXME :: We really can not legally get a new page/bh @@ -677,10 +675,8 @@ * BH walking or segment can only be updated after we have a * good hwif->INB(IDE_STATUS_REG); return. */ - if (!rq->current_nr_sectors) { - if (!DRIVER(drive)->end_request(drive, 1, 0)) - return ide_stopped; - } + if (!DRIVER(drive)->end_request(drive, 1, 1)) + return ide_stopped; } while (msect); if (HWGROUP(drive)->handler == NULL) ide_set_handler(drive, &task_mulin_intr, WAIT_WORSTCASE, NULL); @@ -740,9 +736,9 @@ * Safe to update request for partial completions. * We have a good STATUS CHECK!!! */ - if (!rq->current_nr_sectors) - if (!DRIVER(drive)->end_request(drive, 1, 0)) - return ide_stopped; + if (!DRIVER(drive)->end_request(drive, 1, 1)) + return ide_stopped; + if ((rq->current_nr_sectors==1) ^ (stat & DRQ_STAT)) { rq = HWGROUP(drive)->rq; pBuf = task_map_rq(rq, &flags); @@ -802,13 +798,10 @@ msect -= nsect; taskfile_output_data(drive, pBuf, nsect * SECTOR_WORDS); task_unmap_rq(rq, pBuf, &flags); - rq->current_nr_sectors -= nsect; - if (!rq->current_nr_sectors) { - if (!DRIVER(drive)->end_request(drive, 1, 0)) - if (!rq->bio) { - stat = hwif->INB(IDE_STATUS_REG); - return ide_stopped; - } + if (!DRIVER(drive)->end_request(drive, 1, 1)) { + /* stat for...? */ + stat = hwif->INB(IDE_STATUS_REG); + return ide_stopped; } } while (msect); rq->errors = 0; @@ -922,18 +915,14 @@ msect -= nsect; taskfile_output_data(drive, pBuf, nsect * SECTOR_WORDS); task_unmap_rq(rq, pBuf, &flags); - rq->current_nr_sectors -= nsect; /* * FIXME :: We really can not legally get a new page/bh * regardless, if this is the end of our segment. * BH walking or segment can only be updated after we * have a good hwif->INB(IDE_STATUS_REG); return. */ - if (!rq->current_nr_sectors) { - if (!DRIVER(drive)->end_request(drive, 1, 0)) - if (!rq->bio) - return ide_stopped; - } + if (!DRIVER(drive)->end_request(drive, 1, 1)) + return ide_stopped; } while (msect); rq->errors = 0; if (HWGROUP(drive)->handler == NULL) -- 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/