Subject: [ANNOUNCE] 2.5.66 bio traversal + IDE PIO patches on the way


Hey,

I have ported Suparna's 2.5.30 bio traversal patches to 2.5.66
dropping bi_voffset+bi_endvoffset code.

Just to remind - bio traversing adds possibility to traverse
bio list partially for i/o submission without affecting completion.
For more detailed info just read second patch (documentation update).

Next patches implement new handlers for IDE PIO.
They use bio traversal and are more correct than current ones.

I have tested them and they are working just fine for me.
If you want to try just apply:

2.5.66-biowalk-core-A0.diff
2.5.66-masked_irq-fix-A0.diff
2.5.66-ide-pio-1-A0.diff
2.5.66-ide-pio-2-A0.diff
and turn on IDE_TASKFILE_IO config option in IDE menu

2.5.66-biowalk-doc-A0.diff
is a documentation update

2.5.66-ide-pio-3-A0.diff
2.5.66-ide-pio-4-A0.diff
are cleanups

As always with block or IDE changes special care is _strongly_
recommended, don't blame me if it eats your fs :-).

--
Bartlomiej Zolnierkiewicz


Subject: [PATCH] IDE 2.5.66-masked_irq-fix-A0


# Revert previous masked_irq handling for ide_do_request().
# Fixes "hda: lost interrupt" bug.
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66/drivers/ide/ide-io.c linux/drivers/ide/ide-io.c
--- linux-2.5.66/drivers/ide/ide-io.c Tue Mar 25 22:53:09 2003
+++ linux/drivers/ide/ide-io.c Tue Mar 25 22:54:33 2003
@@ -850,14 +850,14 @@
* happens anyway when any interrupt comes in, IDE or otherwise
* -- the kernel masks the IRQ while it is being handled.
*/
- if (hwif->irq != masked_irq)
+ if (masked_irq && hwif->irq != masked_irq)
disable_irq_nosync(hwif->irq);
spin_unlock(&ide_lock);
local_irq_enable();
/* allow other IRQs while we start this request */
startstop = start_request(drive, rq);
spin_lock_irq(&ide_lock);
- if (hwif->irq != masked_irq)
+ if (masked_irq && hwif->irq != masked_irq)
enable_irq(hwif->irq);
if (startstop == ide_released)
goto queue_next;
@@ -873,7 +873,7 @@
*/
void do_ide_request(request_queue_t *q)
{
- ide_do_request(q->queuedata, IDE_NO_IRQ);
+ ide_do_request(q->queuedata, 0);
}

/*
@@ -1021,7 +1021,7 @@
hwgroup->busy = 0;
}
}
- ide_do_request(hwgroup, IDE_NO_IRQ);
+ ide_do_request(hwgroup, 0);
spin_unlock_irqrestore(&ide_lock, flags);
}

@@ -1311,7 +1311,7 @@
insert_end = 0;
}
__elv_add_request(&drive->queue, rq, insert_end, 0);
- ide_do_request(hwgroup, IDE_NO_IRQ);
+ ide_do_request(hwgroup, 0);
spin_unlock_irqrestore(&ide_lock, flags);

err = 0;


Subject: [PATCH] bio traversal 1/2 - core changes


# Bio traversal (separate submittion/completion bio pointers).
# Patch 1/2 - Core changes.
#
# Originally by Suparna Bhattacharya.
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66/drivers/block/elevator.c linux/drivers/block/elevator.c
--- linux-2.5.66/drivers/block/elevator.c Fri Mar 21 14:23:25 2003
+++ linux/drivers/block/elevator.c Sat Mar 22 22:50:06 2003
@@ -324,6 +324,19 @@
if (&rq->queuelist == q->last_merge)
q->last_merge = NULL;

+ /*
+ * preset some fields (e.g. the req might have been restarted)
+ */
+ if ((rq->bio = rq->hard_bio)) {
+ rq->nr_bio_segments = bio_segments(rq->bio);
+ rq->nr_bio_sectors = bio_sectors(rq->bio);
+ rq->hard_cur_sectors = bio_cur_sectors(rq->bio);
+ rq->buffer = bio_data(rq->bio);
+ }
+ rq->sector = rq->hard_sector;
+ rq->nr_sectors = rq->hard_nr_sectors;
+ rq->current_nr_sectors = rq->hard_cur_sectors;
+
if ((rq->flags & REQ_DONTPREP) || !q->prep_rq_fn)
break;

diff -uNr linux-2.5.66/drivers/block/ll_rw_blk.c linux/drivers/block/ll_rw_blk.c
--- linux-2.5.66/drivers/block/ll_rw_blk.c Tue Mar 25 22:53:09 2003
+++ linux/drivers/block/ll_rw_blk.c Tue Mar 25 23:51:01 2003
@@ -1680,7 +1680,8 @@

sector = bio->bi_sector;
nr_sectors = bio_sectors(bio);
- cur_nr_sectors = bio_iovec(bio)->bv_len >> 9;
+ cur_nr_sectors = bio_cur_sectors(bio);
+
rw = bio_data_dir(bio);

/*
@@ -1736,7 +1737,10 @@
}

bio->bi_next = req->bio;
- req->bio = bio;
+ req->hard_bio = req->bio = bio;
+ req->nr_bio_segments = bio_segments(req->bio);
+ req->nr_bio_sectors = bio_sectors(req->bio);
+
/*
* may not be valid. if the low level driver said
* it didn't need a bounce buffer then it better
@@ -1806,7 +1810,9 @@
req->nr_hw_segments = bio_hw_segments(q, bio);
req->buffer = bio_data(bio); /* see ->buffer comment above */
req->waiting = NULL;
- req->bio = req->biotail = bio;
+ req->hard_bio = req->bio = req->biotail = bio;
+ req->nr_bio_segments = bio_segments(req->bio);
+ req->nr_bio_sectors = bio_sectors(req->bio);
req->rq_disk = bio->bi_bdev->bd_disk;
req->start_time = jiffies;
add_request(q, req, insert_here);
@@ -1985,10 +1991,8 @@
if (!rq->bio)
return;

- rq->buffer = bio_data(rq->bio);
-
nr_phys_segs = nr_hw_segs = 0;
- rq_for_each_bio(bio, rq) {
+ rq_for_each_hard_bio(bio, rq) {
/* Force bio hw/phys segs to be recalculated. */
bio->bi_flags &= ~(1 << BIO_SEG_VALID);

@@ -2004,11 +2008,24 @@
{
if (blk_fs_request(rq)) {
rq->hard_sector += nsect;
- rq->nr_sectors = rq->hard_nr_sectors -= nsect;
- rq->sector = rq->hard_sector;
+ rq->hard_nr_sectors -= nsect;

- rq->current_nr_sectors = bio_iovec(rq->bio)->bv_len >> 9;
- rq->hard_cur_sectors = rq->current_nr_sectors;
+ /*
+ * Move the I/O submission pointers ahead if required
+ * (i.e. if the driver doesn't update them).
+ */
+ if ((rq->nr_sectors >= rq->hard_nr_sectors) &&
+ (rq->sector <= rq->hard_sector)) {
+ rq->sector = rq->hard_sector;
+ rq->nr_sectors = rq->hard_nr_sectors;
+ rq->bio = rq->hard_bio;
+ rq->nr_bio_segments = bio_segments(rq->bio);
+ rq->nr_bio_sectors = bio_sectors(rq->bio);
+ rq->hard_cur_sectors = bio_cur_sectors(rq->bio);
+ rq->current_nr_sectors = rq->hard_cur_sectors;
+
+ rq->buffer = bio_data(rq->bio);
+ }

/*
* if total number of sectors is less than the first segment
@@ -2043,11 +2060,11 @@
}

total_bytes = bio_nbytes = 0;
- while ((bio = req->bio)) {
+ while ((bio = req->hard_bio)) {
int nbytes;

if (nr_bytes >= bio->bi_size) {
- req->bio = bio->bi_next;
+ req->hard_bio = bio->bi_next;
nbytes = bio->bi_size;
bio_endio(bio, nbytes, error);
next_idx = 0;
@@ -2087,7 +2104,7 @@
total_bytes += nbytes;
nr_bytes -= nbytes;

- if ((bio = req->bio)) {
+ if ((bio = req->hard_bio)) {
/*
* end more in this run, or just return 'not-done'
*/
@@ -2099,7 +2116,7 @@
/*
* completely done
*/
- if (!req->bio)
+ if (!req->hard_bio)
return 0;

/*
@@ -2107,7 +2124,7 @@
*/
if (bio_nbytes) {
bio_endio(bio, bio_nbytes, error);
- req->bio->bi_idx += next_idx;
+ req->hard_bio->bi_idx += next_idx;
}

blk_recalc_rq_sectors(req, total_bytes >> 9);
@@ -2181,6 +2198,80 @@
__blk_put_request(req->q, req);
}

+/**
+ * blk_rq_next_segment
+ * @rq: the request being processed
+ *
+ * Description:
+ * Points to the next segment in the request if the current segment
+ * is complete. Leaves things unchanged if this segment is not over
+ * or if no more segments are left in this request.
+ *
+ * Meant to be used for bio traversal during I/O submission
+ * Does not effect any I/O completions or update completion state
+ * in the request, and does not modify any bio fields.
+ *
+ * Decrementing rq->nr_sectors, rq->current_nr_sectors and
+ * rq->nr_bio_sectors as data is transferred is the caller's
+ * responsibility and should be done before calling this routine.
+ **/
+void blk_rq_next_segment(struct request *rq)
+{
+ if (rq->current_nr_sectors > 0)
+ return;
+
+ if (rq->nr_bio_sectors > 0) {
+ --rq->nr_bio_segments;
+ rq->current_nr_sectors = blk_rq_vec(rq)->bv_len >> 9;
+ } else {
+ if ((rq->bio = rq->bio->bi_next)) {
+ rq->nr_bio_segments = bio_segments(rq->bio);
+ rq->nr_bio_sectors = bio_sectors(rq->bio);
+ rq->current_nr_sectors = bio_cur_sectors(rq->bio);
+ }
+ }
+
+ /* remember the size of this segment before we start I/O */
+ rq->hard_cur_sectors = rq->current_nr_sectors;
+}
+
+/**
+ * process_that_request_first - process partial request submission
+ * @req: the request being processed
+ * @nr_sectors: number of sectors I/O has been submitted on
+ *
+ * Description:
+ * May be used for processing bio's while submitting I/O without
+ * signalling completion. Fails if more data is requested than is
+ * available in the request in which case it doesn't advance any
+ * pointers.
+ *
+ * Assumes a request is correctly set up. No sanity checks.
+ *
+ * Return:
+ * 0 - no more data left to submit (not processed)
+ * 1 - data available to submit for this request (processed)
+ **/
+int process_that_request_first(struct request *req, unsigned int nr_sectors)
+{
+ unsigned int nsect;
+
+ if (req->nr_sectors < nr_sectors)
+ return 0;
+
+ req->nr_sectors -= nr_sectors;
+ req->sector += nr_sectors;
+ while (nr_sectors) {
+ nsect = min_t(unsigned int, req->current_nr_sectors,
+ nr_sectors);
+ req->current_nr_sectors -= nsect;
+ req->nr_bio_sectors -= nsect;
+ nr_sectors -= nsect;
+ blk_rq_next_segment(req);
+ }
+ return 1;
+}
+
int __init blk_dev_init(void)
{
int total_ram = nr_free_pages() << (PAGE_SHIFT - 10);
@@ -2223,6 +2314,7 @@
EXPORT_SYMBOL(end_that_request_first);
EXPORT_SYMBOL(end_that_request_chunk);
EXPORT_SYMBOL(end_that_request_last);
+EXPORT_SYMBOL(process_that_request_first);
EXPORT_SYMBOL(blk_init_queue);
EXPORT_SYMBOL(blk_cleanup_queue);
EXPORT_SYMBOL(blk_queue_make_request);
diff -uNr linux-2.5.66/include/linux/bio.h linux/include/linux/bio.h
--- linux-2.5.66/include/linux/bio.h Fri Mar 21 14:24:36 2003
+++ linux/include/linux/bio.h Sat Mar 22 22:52:16 2003
@@ -131,6 +131,7 @@
#define bio_iovec(bio) bio_iovec_idx((bio), (bio)->bi_idx)
#define bio_page(bio) bio_iovec((bio))->bv_page
#define bio_offset(bio) bio_iovec((bio))->bv_offset
+#define bio_segments(bio) ((bio)->bi_vcnt - (bio)->bi_idx)
#define bio_sectors(bio) ((bio)->bi_size >> 9)
#define bio_cur_sectors(bio) (bio_iovec(bio)->bv_len >> 9)
#define bio_data(bio) (page_address(bio_page((bio))) + bio_offset((bio)))
@@ -225,12 +226,15 @@
#ifdef CONFIG_HIGHMEM
/*
* remember to add offset! and never ever reenable interrupts between a
- * bio_kmap_irq and bio_kunmap_irq!!
+ * bvec_kmap_irq and bvec_kunmap_irq!!
*
* This function MUST be inlined - it plays with the CPU interrupt flags.
* Hence the `extern inline'.
+ *
+ * We use bio_vec * directly here instead of bio_page(bio) because we also
+ * want to use this function for bio_vec's different than bio->bi_idx one.
*/
-extern inline char *bio_kmap_irq(struct bio *bio, unsigned long *flags)
+extern inline char *bvec_kmap_irq(struct bio_vec *bvec, unsigned long *flags)
{
unsigned long addr;

@@ -239,15 +243,15 @@
* balancing is a lot nicer this way
*/
local_save_flags(*flags);
- addr = (unsigned long) kmap_atomic(bio_page(bio), KM_BIO_SRC_IRQ);
+ addr = (unsigned long) kmap_atomic(bvec->bv_page, KM_BIO_SRC_IRQ);

if (addr & ~PAGE_MASK)
BUG();

- return (char *) addr + bio_offset(bio);
+ return (char *) addr + bvec->bv_offset;
}

-extern inline void bio_kunmap_irq(char *buffer, unsigned long *flags)
+extern inline void bvec_kunmap_irq(char *buffer, unsigned long *flags)
{
unsigned long ptr = (unsigned long) buffer & PAGE_MASK;

@@ -256,8 +260,20 @@
}

#else
-#define bio_kmap_irq(bio, flags) (bio_data(bio))
-#define bio_kunmap_irq(buf, flags) do { *(flags) = 0; } while (0)
+#define bvec_kmap_irq(bvec, flags) (page_address((bvec)->bv_page) + (bvec)->bv_offset)
+#define bvec_kunmap_irq(buf, flags) do { *(flags) = 0; } while (0)
#endif

+extern inline char *__bio_kmap_irq(struct bio *bio, unsigned short idx,
+ unsigned long *flags)
+{
+ return bvec_kmap_irq(bio_iovec_idx(bio, idx), flags);
+}
+#define __bio_kunmap_irq bvec_kunmap_irq
+
+/* Will be removed soon. */
+#define bio_kmap_irq(bio, flags) \
+ __bio_kmap_irq((bio), (bio)->bi_idx, (flags))
+#define bio_kunmap_irq __bio_kunmap_irq
+
#endif /* __LINUX_BIO_H */
diff -uNr linux-2.5.66/include/linux/blk.h linux/include/linux/blk.h
--- linux-2.5.66/include/linux/blk.h Fri Mar 21 14:23:25 2003
+++ linux/include/linux/blk.h Sat Mar 22 22:50:06 2003
@@ -43,6 +43,7 @@
extern int end_that_request_first(struct request *, int, int);
extern int end_that_request_chunk(struct request *, int, int);
extern void end_that_request_last(struct request *);
+extern int process_that_request_first(struct request *, unsigned int);
struct request *elv_next_request(request_queue_t *q);

static inline void blkdev_dequeue_request(struct request *req)
diff -uNr linux-2.5.66/include/linux/blkdev.h linux/include/linux/blkdev.h
--- linux-2.5.66/include/linux/blkdev.h Fri Mar 21 14:23:25 2003
+++ linux/include/linux/blkdev.h Sat Mar 22 22:50:06 2003
@@ -10,6 +10,7 @@
#include <linux/pagemap.h>
#include <linux/backing-dev.h>
#include <linux/wait.h>
+#include <linux/bio.h>

#include <asm/scatterlist.h>

@@ -33,9 +34,28 @@
* blkdev_dequeue_request! */
unsigned long flags; /* see REQ_ bits below */

- sector_t sector;
- unsigned long nr_sectors;
+ /* Maintain bio traversal state for part by part I/O submission.
+ * hard_* are block layer internals, no driver should touch them!
+ */
+
+ sector_t sector; /* next sector to submit */
+ unsigned long nr_sectors; /* no. of sectors left to submit */
+
+ sector_t hard_sector; /* next sector to complete */
+ unsigned long hard_nr_sectors; /* no. of sectors left to complete */
+
+ /* no. of segments left to submit in the current bio */
+ unsigned short nr_bio_segments;
+ /* no. of sectors left to submit in the current bio */
+ unsigned long nr_bio_sectors;
+ /* no. of sectors left to submit in the current segment */
unsigned int current_nr_sectors;
+ /* no. of sectors left to complete in the current segment */
+ unsigned int hard_cur_sectors;
+
+ struct bio *bio; /* next bio to submit */
+ struct bio *hard_bio; /* next unfinished bio to complete */
+ struct bio *biotail;

void *elevator_private;

@@ -43,15 +63,6 @@
struct gendisk *rq_disk;
int errors;
unsigned long start_time;
- sector_t hard_sector; /* the hard_* are block layer
- * internals, no driver should
- * touch them
- */
- unsigned long hard_nr_sectors;
- unsigned int hard_cur_sectors;
-
- struct bio *bio;
- struct bio *biotail;

/* Number of scatter-gather DMA addr+len pairs after
* physical address coalescing is performed.
@@ -282,6 +293,32 @@
*/
#define blk_queue_headactive(q, head_active)

+/* current index into bio being processed for submission */
+#define blk_rq_idx(rq) ((rq)->bio->bi_vcnt - (rq)->nr_bio_segments)
+
+/* current bio vector being processed */
+#define blk_rq_vec(rq) (bio_iovec_idx((rq)->bio, blk_rq_idx(rq)))
+
+/*
+ * temporarily mapping a (possible) highmem bio (typically for PIO transfer)
+ */
+
+/* current offset with respect to start of the segment being submitted */
+#define blk_rq_offset(rq) \
+ (((rq)->hard_cur_sectors - (rq)->current_nr_sectors) << 9)
+
+/* Assumes rq->bio != NULL */
+static inline char * rq_map_buffer(struct request *rq, unsigned long *flags)
+{
+ return (__bio_kmap_irq(rq->bio, blk_rq_idx(rq), flags)
+ + blk_rq_offset(rq));
+}
+
+static inline void rq_unmap_buffer(char *buffer, unsigned long *flags)
+{
+ __bio_kunmap_irq(buffer, flags);
+}
+
/*
* q->prep_rq_fn return values
*/
@@ -319,6 +356,10 @@
if ((rq->bio)) \
for (bio = (rq)->bio; bio; bio = bio->bi_next)

+#define rq_for_each_hard_bio(bio, rq) \
+ if ((rq->hard_bio)) \
+ for (bio = (rq)->hard_bio; bio; bio = bio->bi_next)
+
struct sec_size {
unsigned block_size;
unsigned block_size_bits;
diff -uNr linux-2.5.66/mm/highmem.c linux/mm/highmem.c
--- linux-2.5.66/mm/highmem.c Tue Mar 25 22:53:11 2003
+++ linux/mm/highmem.c Tue Mar 25 23:51:01 2003
@@ -431,7 +431,7 @@
bio->bi_rw = (*bio_orig)->bi_rw;

bio->bi_vcnt = (*bio_orig)->bi_vcnt;
- bio->bi_idx = 0;
+ bio->bi_idx = (*bio_orig)->bi_idx;
bio->bi_size = (*bio_orig)->bi_size;

if (pool == page_pool) {


Subject: [PATCH] bio traversal 2/2 - documentation changes


# Bio traversal (separate submittion/completion bio pointers).
# Patch 2/2 - Documentation changes.
#
# Originally by Suparna Bhattacharya.
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66/Documentation/block/biodoc.txt linux/Documentation/block/biodoc.txt
--- linux-2.5.66/Documentation/block/biodoc.txt Mon Mar 17 19:49:00 2003
+++ linux/Documentation/block/biodoc.txt Tue Mar 18 21:42:40 2003
@@ -5,7 +5,7 @@
Jens Axboe <[email protected]>
Suparna Bhattacharya <[email protected]>

-Last Updated May 2, 2002
+Last Updated March 18, 2003

Introduction:

@@ -204,8 +204,8 @@
which case a virtual mapping of the page is required. For SCSI it is also
done in some scenarios where the low level driver cannot be trusted to
handle a single sg entry correctly. The driver is expected to perform the
-kmaps as needed on such occasions using the bio_kmap and bio_kmap_irq
-routines as appropriate. A driver could also use the blk_queue_bounce()
+kmaps as needed on such occasions using the rq_map_buffer() routine
+as appropriate. A driver could also use the blk_queue_bounce()
routine on its own to bounce highmem i/o to low memory for specific requests
if so desired.

@@ -399,7 +399,8 @@
directly by hand.
This is because end_that_request_first only iterates over the bio list,
and always returns 0 if there are none associated with the request.
- _last works OK in this case, and is not a problem, as I mentioned earlier
+ end_that_request_last works OK in this case, and is not a problem,
+ as mentioned earlier.
>

1.3.1 Pre-built Commands
@@ -609,11 +610,20 @@
unsigned short nr_hw_segments;

/* Various sector counts */
+ /*
+ * The various block internal copies represent counts/pointers of
+ * unfinished I/O, while the other counts/pointers refer to
+ * I/O to be submitted.
+ */
unsigned long nr_sectors; /* no. of sectors left: driver modifiable */
unsigned long hard_nr_sectors; /* block internal copy of above */
unsigned int current_nr_sectors; /* no. of sectors left in the
current segment:driver modifiable */
unsigned long hard_cur_sectors; /* block internal copy of the above */
+
+ unsigned short nr_bio_segments; /* no. of segments left in curr bio */
+ unsigned short nr_bio_sectors; /* no. of sectors left in curr bio */
+
.
.
int tag; /* command tag associated with request */
@@ -623,6 +633,7 @@
.
.
struct bio *bio, *biotail; /* bio list instead of bh */
+ struct bio *hard_bio; /* block internal copy */
struct request_list *rl;
}

@@ -641,9 +652,11 @@
transfer and invokes block end*request helpers to mark this. The
driver should not modify these values. The block layer sets up the
nr_sectors and current_nr_sectors fields (based on the corresponding
-hard_xxx values and the number of bytes transferred) and updates it on
-every transfer that invokes end_that_request_first. It does the same for the
-buffer, bio, bio->bi_idx fields too.
+hard_xxx values and the number of bytes transferred) and typically
+updates it on every transfer that invokes end_that_request_first,
+unless the driver has advanced these (submission) counters ahead
+of the sectors being completed. The block layer also advances the
+buffer, bio, bio->bi_idx fields appropriately as well as i/o completes.

The buffer field is just a virtual address mapping of the current segment
of the i/o buffer in cases where the buffer resides in low-memory. For high
@@ -653,6 +666,60 @@
a driver needs to be careful about interoperation with the block layer helper
functions which the driver uses. (Section 1.3)

+2.3.1 The Separation of Submission and Completion State
+
+The basic protocol followed all through is that the bio fields
+would always reflect the status w.r.t how much i/o remains
+to be completed. On the other hand submission status would
+only be maintained in the request structure. In most cases
+of course, both move in sync (the generic end_that_request_first
+code tries to handle that transparently by advancing the
+submission pointers if they are behind the completion pointers
+as would happen in the case of drivers which don't modify
+those themselves), but for things like IDE mult-count write,
+the submission counters/pointers may be ahead of the
+completion pointers.
+
+The following fields have been added to the request structure
+to help maintain this distinction.
+
+rq->hard_bio
+ the rq->bio field now reflects the next bio which
+is to be submitted for i/o. Hence, the need for rq->hard_bio
+which keeps track of the next bio to be completed (this
+is the one used by end_that_request_first now, instead
+of rq->bio)
+
+rq->nr_bio_segments
+ this keeps track of how many more vecs remain
+to be submitted in the current bio (rq->bio). It is
+used to compute the current index into rq->bio which
+specifies the segment under submission.
+(rq_map_buffer for example uses this field to map
+the right buffer)
+
+rq->nr_bio_sectors
+ this keeps track of the number of sectors to
+be submitted in the current bio (rq->bio). It can be
+used to compute the remaining sectors in the current
+segment in the situation when it is the last segment.
+
+Now a subtle point about hard_cur_sectors. It reflects
+the number of sectors left to be completed in the
+_current_ segment under submission (i.e. the segment
+in rq->bio, and _not_ rq->hard_bio). This makes it
+possible to use it in rq_map_buffer to determine the
+relative offset in the current segment w.r.t what
+the bio indices might indicate.
+
+A new helper, process_that_request_first() has been
+introduced for updating submission state of the request
+without completing the corresponding bios. It can be used
+by code such as mult-count write which need to traverse
+multiple bio segments for each chunk of i/o submitted,
+where multiple such chunk transfers are required to cover
+the entire request.
+
3. Using bios

3.1 Setup/Teardown
@@ -732,6 +799,16 @@
which don't make a distinction between segments and completion units would
need to be reorganized to support multi-segment bios.

+It is recommended that drivers utilize the block layer routine
+process_that_request_first() while traversing bios for i/o submission,
+instead of iterating over the segments directly, and use
+end_that_request_first() for completion as before. Things like
+rq_map_buffer() rely on the submission pointers in the request
+to map the correct buffer.
+
+rq_map_buffer() could be used to get a virtual address mapping
+for the current segment buffer, in drivers which use PIO for example.
+
3.2.2 Setting up DMA scatterlists

The blk_rq_map_sg() helper routine would be used for setting up scatter
@@ -1147,9 +1224,8 @@
PIO drivers (or drivers that need to revert to PIO transfer once in a
while (IDE for example)), where the CPU is doing the actual data
transfer a virtual mapping is needed. If the driver supports highmem I/O,
-(Sec 1.1, (ii) ) it needs to use bio_kmap and bio_kmap_irq to temporarily
-map a bio into the virtual address space. See how IDE handles this with
-ide_map_buffer.
+(Sec 1.1, (ii) ) it needs to use rq_map_buffer() to temporarily
+map a bio into the virtual address space. See how IDE handles this.


8. Prior/Related/Impacted patches
diff -uNr linux-2.5.66/Documentation/block/request.txt linux/Documentation/block/request.txt
--- linux-2.5.66/Documentation/block/request.txt Fri Sep 20 16:19:26 2002
+++ linux/Documentation/block/request.txt Tue Mar 18 21:54:16 2003
@@ -52,11 +52,15 @@

sector_t sector DBI Target location

-unsigned long hard_nr_sectors B Used to keep sector sane
+sector_t hard_sector B Used to keep sector sane
+ Tracks the location of unfinished
+ portion

unsigned long nr_sectors DBI Total number of sectors in request

unsigned long hard_nr_sectors B Used to keep nr_sectors sane
+ Tracks no. of unfinished sectors in
+ the request

unsigned short nr_phys_segments DB Number of physical scatter gather
segments in a request
@@ -68,6 +72,14 @@
of request

unsigned int hard_cur_sectors B Used to keep current_nr_sectors sane
+ Tracks no. unfinished sectors in the
+ same segment.
+
+unsigned long nr_bio_sectors DB Number of unfinished sectors in first
+ bio of request
+
+unsigned short nr_bio_segments DB Number of unfinished segments in first
+ bio of request

int tag DB TCQ tag, if assigned

@@ -79,10 +91,12 @@
struct completion *waiting D Can be used by driver to get signalled
on request completion

-struct bio *bio DBI First bio in request
+struct bio *bio DBI First unsubmitted bio in request

struct bio *biotail DBI Last bio in request

+struct bio *hard_bio B First unfinished bio in request
+
request_queue_t *q DB Request queue this request belongs to

struct request_list *rl B Request list this request came from

Subject: [PATCH] new IDE PIO handlers 1/4


# Rewritten PIO handlers, both single and multiple sector sizes.
#
# They make use of new bio travelsing code and are supposed to be
# correct in respect to ATA state machine.
#
# Patch 1/4 - Implement them, do not activate yet.
# Fix do_rw_taskfile() and flagged_taskfile() for correct
# handling of new prehandlers.
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.66/drivers/ide/ide-taskfile.c Tue Mar 25 22:53:09 2003
+++ linux/drivers/ide/ide-taskfile.c Wed Mar 26 22:49:34 2003
@@ -5,6 +5,7 @@
* Copyright (C) 2000-2002 Andre Hedrick <[email protected]>
* Copyright (C) 2001-2002 Klaus Smolin
* IBM Storage Technology Division
+ * Copyright (C) 2003 Bartlomiej Zolnierkiewicz
*
* The big the bad and the ugly.
*
@@ -176,9 +177,12 @@

hwif->OUTB((taskfile->device_head & HIHI) | drive->select.all, IDE_SELECT_REG);
if (task->handler != NULL) {
- ide_execute_command(drive, taskfile->command, task->handler, WAIT_WORSTCASE, NULL);
- if (task->prehandler != NULL)
+ if (task->prehandler != NULL) {
+ hwif->OUTBSYNC(drive, taskfile->command, IDE_COMMAND_REG);
+ ndelay(400); /* FIXME */
return task->prehandler(drive, task->rq);
+ }
+ ide_execute_command(drive, taskfile->command, task->handler, WAIT_WORSTCASE, NULL);
return ide_started;
}

@@ -535,6 +539,262 @@

EXPORT_SYMBOL(task_no_data_intr);

+#define PIO_IN 0
+#define PIO_OUT 1
+
+static inline void task_sectors(ide_drive_t *drive, struct request *rq,
+ unsigned nsect, int rw)
+{
+ unsigned long flags;
+ char *buf;
+
+ buf = task_map_rq(rq, &flags);
+
+ /*
+ * IRQ can happen instantly after reading/writing
+ * last sector of the datablock.
+ */
+ process_that_request_first(rq, nsect);
+
+ if (rw == PIO_OUT)
+ taskfile_output_data(drive, buf, nsect * SECTOR_WORDS);
+ else
+ taskfile_input_data(drive, buf, nsect * SECTOR_WORDS);
+
+ task_unmap_rq(rq, buf, &flags);
+}
+
+/*
+ * Handler for command with PIO data-in phase (Read).
+ */
+ide_startstop_t task_in_intr (ide_drive_t *drive)
+{
+ struct request *rq = HWGROUP(drive)->rq;
+ u8 stat, good_stat;
+
+ good_stat = DATA_READY;
+check_status:
+ stat = HWIF(drive)->INB(IDE_STATUS_REG);
+ if (!OK_STAT(stat, good_stat, BAD_R_STAT)) {
+ if (stat & (ERR_STAT|DRQ_STAT))
+ return DRIVER(drive)->error(drive, __FUNCTION__, stat);
+ /* No data yet, so wait for another IRQ. */
+ ide_set_handler(drive, &task_in_intr, WAIT_WORSTCASE, NULL);
+ return ide_started;
+ }
+
+ /*
+ * Complete previously submitted bios (if any).
+ * Status was already verifyied.
+ */
+ while (rq->hard_bio != rq->bio)
+ if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+ return ide_stopped;
+
+ rq->errors = 0;
+ task_sectors(drive, rq, 1, PIO_IN);
+
+ /* If it was the last datablock check status and finish transfer. */
+ if (!rq->nr_sectors) {
+ good_stat = 0;
+ goto check_status;
+ }
+
+ /* Still data left to transfer. */
+ ide_set_handler(drive, &task_in_intr, WAIT_WORSTCASE, NULL);
+
+ return ide_started;
+}
+
+EXPORT_SYMBOL(task_in_intr);
+
+/*
+ * Handler for command with PIO data-in phase (Read Multiple).
+ */
+ide_startstop_t task_mulin_intr (ide_drive_t *drive)
+{
+ struct request *rq = HWGROUP(drive)->rq;
+ unsigned int msect = drive->mult_count;
+ unsigned int nsect;
+ u8 stat, good_stat;
+
+ good_stat = DATA_READY;
+check_status:
+ stat = HWIF(drive)->INB(IDE_STATUS_REG);
+ if (!OK_STAT(stat, good_stat, BAD_R_STAT)) {
+ if (stat & (ERR_STAT|DRQ_STAT))
+ return DRIVER(drive)->error(drive, __FUNCTION__, stat);
+ /* No data yet, so wait for another IRQ. */
+ ide_set_handler(drive, &task_mulin_intr, WAIT_WORSTCASE, NULL);
+ return ide_started;
+ }
+
+ /*
+ * Complete previously submitted bios (if any).
+ * Status was already verifyied.
+ */
+ while (rq->hard_bio != rq->bio)
+ if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+ return ide_stopped;
+
+ rq->errors = 0;
+ do {
+ nsect = rq->current_nr_sectors;
+ if (nsect > msect)
+ nsect = msect;
+
+ task_sectors(drive, rq, nsect, PIO_IN);
+
+ if (!rq->nr_sectors)
+ msect = 0;
+ else
+ msect -= nsect;
+ } while (msect);
+
+ /* If it was the last datablock check status and finish transfer. */
+ if (!rq->nr_sectors) {
+ good_stat = 0;
+ goto check_status;
+ }
+
+ /* Still data left to transfer. */
+ ide_set_handler(drive, &task_mulin_intr, WAIT_WORSTCASE, NULL);
+
+ return ide_started;
+}
+
+EXPORT_SYMBOL(task_mulin_intr);
+
+/*
+ * Handler for command with PIO data-out phase (Write).
+ */
+ide_startstop_t task_out_intr (ide_drive_t *drive)
+{
+ struct request *rq = HWGROUP(drive)->rq;
+ u8 stat;
+ int ok_stat;
+
+ stat = HWIF(drive)->INB(IDE_STATUS_REG);
+ ok_stat = OK_STAT(stat, DRIVE_READY, drive->bad_wstat);
+
+ if (!ok_stat || !rq->nr_sectors) {
+ if (stat & (ERR_STAT|DRQ_STAT))
+ return DRIVER(drive)->error(drive, __FUNCTION__, stat);
+ if (stat & BUSY_STAT) {
+ /* Not ready yet, so wait for another IRQ. */
+ ide_set_handler(drive, &task_out_intr, WAIT_WORSTCASE, NULL);
+ return ide_started;
+ }
+ }
+
+ /*
+ * Complete previously submitted bios (if any).
+ * Status was already verifyied.
+ */
+ while (rq->hard_bio != rq->bio)
+ if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+ return ide_stopped;
+
+ /* Still data left to transfer. */
+ ide_set_handler(drive, &task_out_intr, WAIT_WORSTCASE, NULL);
+
+ rq->errors = 0;
+ task_sectors(drive, rq, 1, PIO_OUT);
+
+ return ide_started;
+}
+
+EXPORT_SYMBOL(task_out_intr);
+
+ide_startstop_t pre_task_out_intr (ide_drive_t *drive, struct request *rq)
+{
+ ide_startstop_t startstop;
+
+ if (ide_wait_stat(&startstop, drive, DATA_READY,
+ drive->bad_wstat, WAIT_DRQ)) {
+ printk(KERN_ERR "%s: no DRQ after issuing WRITE%s\n",
+ drive->name, drive->addressing ? "_EXT" : "");
+ return startstop;
+ }
+
+ return task_out_intr(drive);
+}
+
+EXPORT_SYMBOL(pre_task_out_intr);
+
+/*
+ * Handler for command with PIO data-out phase (Write Multiple).
+ */
+ide_startstop_t task_mulout_intr (ide_drive_t *drive)
+{
+ struct request *rq = HWGROUP(drive)->rq;
+ unsigned int msect = drive->mult_count;
+ unsigned int nsect;
+ u8 stat;
+ int ok_stat;
+
+ stat = HWIF(drive)->INB(IDE_STATUS_REG);
+ ok_stat = OK_STAT(stat, DRIVE_READY, drive->bad_wstat);
+
+ if (!ok_stat || !rq->nr_sectors) {
+ if (stat & (ERR_STAT|DRQ_STAT))
+ return DRIVER(drive)->error(drive, __FUNCTION__, stat);
+ if (stat & BUSY_STAT) {
+ /* Not ready yet, so wait for another IRQ. */
+ ide_set_handler(drive, &task_mulout_intr, WAIT_WORSTCASE, NULL);
+ return ide_started;
+ }
+ }
+
+ /*
+ * Complete previously submitted bios (if any).
+ * Status was already verifyied.
+ */
+ while (rq->hard_bio != rq->bio)
+ if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+ return ide_stopped;
+
+ /* Still data left to transfer. */
+ ide_set_handler(drive, &task_mulout_intr, WAIT_WORSTCASE, NULL);
+
+ rq->errors = 0;
+ do {
+ nsect = rq->current_nr_sectors;
+ if (nsect > msect)
+ nsect = msect;
+
+ task_sectors(drive, rq, nsect, PIO_OUT);
+
+ if (!rq->nr_sectors)
+ msect = 0;
+ else
+ msect -= nsect;
+
+ } while (msect);
+
+ return ide_started;
+}
+
+EXPORT_SYMBOL(task_mulout_intr);
+
+ide_startstop_t pre_task_mulout_intr (ide_drive_t *drive, struct request *rq)
+{
+ ide_startstop_t startstop;
+
+ if (ide_wait_stat(&startstop, drive, DATA_READY,
+ drive->bad_wstat, WAIT_DRQ)) {
+ printk(KERN_ERR "%s: no DRQ after issuing MULTWRITE%s\n",
+ drive->name, drive->addressing ? "_EXT" : "");
+ return startstop;
+ }
+
+ return task_mulout_intr(drive);
+}
+
+EXPORT_SYMBOL(pre_task_mulout_intr);
+
+
+#if 0
/*
* Handler for command with PIO data-in phase, READ
*/
@@ -932,6 +1192,7 @@
}

EXPORT_SYMBOL(task_mulout_intr);
+#endif /* 0 */

/* Called by internal to feature out type of command being called */
//ide_pre_handler_t * ide_pre_handler_parser (task_struct_t *taskfile, hob_struct_t *hobfile)
@@ -1873,10 +2134,14 @@
if (task->handler == NULL)
return ide_stopped;

+ if (task->prehandler != NULL) {
+ hwif->OUTBSYNC(drive, taskfile->command, IDE_COMMAND_REG);
+ ndelay(400); /* FIXME */
+ return task->prehandler(drive, HWGROUP(drive)->rq);
+ }
+
/* Issue the command */
ide_execute_command(drive, taskfile->command, task->handler, WAIT_WORSTCASE, NULL);
- if (task->prehandler != NULL)
- return task->prehandler(drive, HWGROUP(drive)->rq);
}

return ide_started;


Subject: [PATCH] IDE *_dump_status() and *_error() cleanup


Incremental to new PIO handlers (but not related to them),
however should also apply to vanilla 2.5.66.

# Remove duplicates of ide_read_24(), ide_dump_status(),
# try_to_flush_leftover_data() and ide_error().
#
# In ide-disk.c: idedisk_read_24(), idedisk_dump_status(), idedisk_error().
# In ide-taskfile.c: task_read_24(), taskfile_dump_status(),
# task_try_to_flush_leftover_data() and taskfile_error().
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66-ide-pio-4/drivers/ide/ide-disk.c linux/drivers/ide/ide-disk.c
--- linux-2.5.66-ide-pio-4/drivers/ide/ide-disk.c Thu Mar 27 00:10:59 2003
+++ linux/drivers/ide/ide-disk.c Thu Mar 27 23:09:56 2003
@@ -73,14 +73,6 @@

#include "legacy/pdc4030.h"

-static inline u32 idedisk_read_24 (ide_drive_t *drive)
-{
- u8 hcyl = HWIF(drive)->INB(IDE_HCYL_REG);
- u8 lcyl = HWIF(drive)->INB(IDE_LCYL_REG);
- u8 sect = HWIF(drive)->INB(IDE_SECTOR_REG);
- return (hcyl<<16)|(lcyl<<8)|sect;
-}
-
/*
* lba_capacity_is_ok() performs a sanity check on the claimed "lba_capacity"
* value for this drive (from its reported identification information).
@@ -348,163 +340,6 @@

static int do_idedisk_flushcache(ide_drive_t *drive);

-static u8 idedisk_dump_status (ide_drive_t *drive, const char *msg, u8 stat)
-{
- ide_hwif_t *hwif = HWIF(drive);
- unsigned long flags;
- u8 err = 0;
-
- local_irq_set(flags);
- printk("%s: %s: status=0x%02x", drive->name, msg, stat);
-#if FANCY_STATUS_DUMPS
- printk(" { ");
- if (stat & BUSY_STAT)
- printk("Busy ");
- else {
- if (stat & READY_STAT) printk("DriveReady ");
- if (stat & WRERR_STAT) printk("DeviceFault ");
- if (stat & SEEK_STAT) printk("SeekComplete ");
- if (stat & DRQ_STAT) printk("DataRequest ");
- if (stat & ECC_STAT) printk("CorrectedError ");
- if (stat & INDEX_STAT) printk("Index ");
- if (stat & ERR_STAT) printk("Error ");
- }
- printk("}");
-#endif /* FANCY_STATUS_DUMPS */
- printk("\n");
- if ((stat & (BUSY_STAT|ERR_STAT)) == ERR_STAT) {
- err = hwif->INB(IDE_ERROR_REG);
- printk("%s: %s: error=0x%02x", drive->name, msg, err);
-#if FANCY_STATUS_DUMPS
- printk(" { ");
- if (err & ABRT_ERR) printk("DriveStatusError ");
- if (err & ICRC_ERR)
- printk("Bad%s ", (err & ABRT_ERR) ? "CRC" : "Sector");
- if (err & ECC_ERR) printk("UncorrectableError ");
- if (err & ID_ERR) printk("SectorIdNotFound ");
- if (err & TRK0_ERR) printk("TrackZeroNotFound ");
- if (err & MARK_ERR) printk("AddrMarkNotFound ");
- printk("}");
- if ((err & (BBD_ERR | ABRT_ERR)) == BBD_ERR ||
- (err & (ECC_ERR|ID_ERR|MARK_ERR))) {
- if (drive->addressing == 1) {
- __u64 sectors = 0;
- u32 low = 0, high = 0;
- low = idedisk_read_24(drive);
- hwif->OUTB(drive->ctl|0x80, IDE_CONTROL_REG);
- high = idedisk_read_24(drive);
- sectors = ((__u64)high << 24) | low;
- printk(", LBAsect=%llu, high=%d, low=%d",
- (unsigned long long) sectors,
- high, low);
- } else {
- u8 cur = hwif->INB(IDE_SELECT_REG);
- if (cur & 0x40) { /* using LBA? */
- printk(", LBAsect=%ld", (unsigned long)
- ((cur&0xf)<<24)
- |(hwif->INB(IDE_HCYL_REG)<<16)
- |(hwif->INB(IDE_LCYL_REG)<<8)
- | hwif->INB(IDE_SECTOR_REG));
- } else {
- printk(", CHS=%d/%d/%d",
- (hwif->INB(IDE_HCYL_REG)<<8) +
- hwif->INB(IDE_LCYL_REG),
- cur & 0xf,
- hwif->INB(IDE_SECTOR_REG));
- }
- }
- if (HWGROUP(drive) && HWGROUP(drive)->rq)
- printk(", sector=%llu",
- (unsigned long long)HWGROUP(drive)->rq->sector);
- }
- }
-#endif /* FANCY_STATUS_DUMPS */
- printk("\n");
- local_irq_restore(flags);
- return err;
-}
-
-ide_startstop_t idedisk_error (ide_drive_t *drive, const char *msg, u8 stat)
-{
- ide_hwif_t *hwif;
- struct request *rq;
- u8 err;
- int i = (drive->mult_count ? drive->mult_count : 1) * SECTOR_WORDS;
-
- err = idedisk_dump_status(drive, msg, stat);
-
- if (drive == NULL || (rq = HWGROUP(drive)->rq) == NULL)
- return ide_stopped;
-
- hwif = HWIF(drive);
- /* retry only "normal" I/O: */
- if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASK | REQ_DRIVE_TASKFILE)) {
- rq->errors = 1;
- ide_end_drive_cmd(drive, stat, err);
- return ide_stopped;
- }
-#if 0
- else if (rq->flags & REQ_DRIVE_TASKFILE) {
- rq->errors = 1;
- ide_end_taskfile(drive, stat, err);
- return ide_stopped;
- }
-#endif
- if (stat & BUSY_STAT || ((stat & WRERR_STAT) && !drive->nowerr)) {
- /* other bits are useless when BUSY */
- rq->errors |= ERROR_RESET;
- } else if (stat & ERR_STAT) {
- /* err has different meaning on cdrom and tape */
- if (err == ABRT_ERR) {
- if (drive->select.b.lba &&
- /* some newer drives don't support WIN_SPECIFY */
- hwif->INB(IDE_COMMAND_REG) == WIN_SPECIFY)
- return ide_stopped;
- } else if ((err & BAD_CRC) == BAD_CRC) {
- /* UDMA crc error, just retry the operation */
- drive->crc_count++;
- } else if (err & (BBD_ERR | ECC_ERR)) {
- /* retries won't help these */
- rq->errors = ERROR_MAX;
- } else if (err & TRK0_ERR) {
- /* help it find track zero */
- rq->errors |= ERROR_RECAL;
- }
- }
- if ((stat & DRQ_STAT) && rq_data_dir(rq) == READ) {
- /*
- * try_to_flush_leftover_data() is invoked in response to
- * a drive unexpectedly having its DRQ_STAT bit set. As
- * an alternative to resetting the drive, this routine
- * tries to clear the condition by read a sector's worth
- * of data from the drive. Of course, this may not help
- * if the drive is *waiting* for data from *us*.
- */
- while (i > 0) {
- u32 buffer[16];
- unsigned int wcount = (i > 16) ? 16 : i;
- i -= wcount;
- taskfile_input_data(drive, buffer, wcount);
- }
- }
- if (hwif->INB(IDE_STATUS_REG) & (BUSY_STAT|DRQ_STAT)) {
- /* force an abort */
- hwif->OUTB(WIN_IDLEIMMEDIATE,IDE_COMMAND_REG);
- }
- if (rq->errors >= ERROR_MAX)
- DRIVER(drive)->end_request(drive, 0, 0);
- else {
- if ((rq->errors & ERROR_RESET) == ERROR_RESET) {
- ++rq->errors;
- return ide_do_reset(drive);
- }
- if ((rq->errors & ERROR_RECAL) == ERROR_RECAL)
- drive->special.b.recalibrate = 1;
- ++rq->errors;
- }
- return ide_stopped;
-}
-
ide_startstop_t idedisk_abort(ide_drive_t *drive, const char *msg)
{
ide_hwif_t *hwif;
@@ -1248,8 +1083,6 @@
.cleanup = idedisk_cleanup,
.flushcache = do_idedisk_flushcache,
.do_request = do_rw_disk,
- .sense = idedisk_dump_status,
- .error = idedisk_error,
.abort = idedisk_abort,
.pre_reset = idedisk_pre_reset,
.capacity = idedisk_capacity,
diff -uNr linux-2.5.66-ide-pio-4/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.66-ide-pio-4/drivers/ide/ide-taskfile.c Thu Mar 27 00:08:16 2003
+++ linux/drivers/ide/ide-taskfile.c Thu Mar 27 23:08:29 2003
@@ -62,15 +62,6 @@
#define task_map_rq(rq, flags) ide_map_buffer((rq), (flags))
#define task_unmap_rq(rq, buf, flags) ide_unmap_buffer((rq), (buf), (flags))

-inline u32 task_read_24 (ide_drive_t *drive)
-{
- return (HWIF(drive)->INB(IDE_HCYL_REG)<<16) |
- (HWIF(drive)->INB(IDE_LCYL_REG)<<8) |
- HWIF(drive)->INB(IDE_SECTOR_REG);
-}
-
-EXPORT_SYMBOL(task_read_24);
-
static void ata_bswap_data (void *buffer, int wcount)
{
u16 *p = buffer;
@@ -220,88 +211,6 @@
EXPORT_SYMBOL(do_rw_taskfile);

/*
- * Error reporting, in human readable form (luxurious, but a memory hog).
- */
-u8 taskfile_dump_status (ide_drive_t *drive, const char *msg, u8 stat)
-{
- ide_hwif_t *hwif = HWIF(drive);
- unsigned long flags;
- u8 err = 0;
-
- local_irq_set(flags);
- printk("%s: %s: status=0x%02x", drive->name, msg, stat);
-#if FANCY_STATUS_DUMPS
- printk(" { ");
- if (stat & BUSY_STAT) {
- printk("Busy ");
- } else {
- if (stat & READY_STAT) printk("DriveReady ");
- if (stat & WRERR_STAT) printk("DeviceFault ");
- if (stat & SEEK_STAT) printk("SeekComplete ");
- if (stat & DRQ_STAT) printk("DataRequest ");
- if (stat & ECC_STAT) printk("CorrectedError ");
- if (stat & INDEX_STAT) printk("Index ");
- if (stat & ERR_STAT) printk("Error ");
- }
- printk("}");
-#endif /* FANCY_STATUS_DUMPS */
- printk("\n");
- if ((stat & (BUSY_STAT|ERR_STAT)) == ERR_STAT) {
- err = hwif->INB(IDE_ERROR_REG);
- printk("%s: %s: error=0x%02x", drive->name, msg, err);
-#if FANCY_STATUS_DUMPS
- if (drive->media == ide_disk)
- goto media_out;
-
- printk(" { ");
- if (err & ABRT_ERR) printk("DriveStatusError ");
- if (err & ICRC_ERR) printk("Bad%s", (err & ABRT_ERR) ? "CRC " : "Sector ");
- if (err & ECC_ERR) printk("UncorrectableError ");
- if (err & ID_ERR) printk("SectorIdNotFound ");
- if (err & TRK0_ERR) printk("TrackZeroNotFound ");
- if (err & MARK_ERR) printk("AddrMarkNotFound ");
- printk("}");
- if ((err & (BBD_ERR | ABRT_ERR)) == BBD_ERR ||
- (err & (ECC_ERR|ID_ERR|MARK_ERR))) {
- if (drive->addressing == 1) {
- u64 sectors = 0;
- u32 high = 0;
- u32 low = task_read_24(drive);
- hwif->OUTB(0x80, IDE_CONTROL_REG);
- high = task_read_24(drive);
- sectors = ((u64)high << 24) | low;
- printk(", LBAsect=%lld", (long long) sectors);
- } else {
- u8 cur = hwif->INB(IDE_SELECT_REG);
- u8 low = hwif->INB(IDE_LCYL_REG);
- u8 high = hwif->INB(IDE_HCYL_REG);
- u8 sect = hwif->INB(IDE_SECTOR_REG);
- /* using LBA? */
- if (cur & 0x40) {
- printk(", LBAsect=%d", (u32)
- ((cur&0xf)<<24)|(high<<16)|
- (low<<8)|sect);
- } else {
- printk(", CHS=%d/%d/%d",
- ((high<<8) + low),
- (cur & 0xf), sect);
- }
- }
- if (HWGROUP(drive)->rq)
- printk(", sector=%llu",
- (unsigned long long)HWGROUP(drive)->rq->sector);
- }
-media_out:
-#endif /* FANCY_STATUS_DUMPS */
- printk("\n");
- }
- local_irq_restore(flags);
- return err;
-}
-
-EXPORT_SYMBOL(taskfile_dump_status);
-
-/*
* Clean up after success/failure of an explicit taskfile operation.
*/
void ide_end_taskfile (ide_drive_t *drive, u8 stat, u8 err)
@@ -362,99 +271,6 @@
EXPORT_SYMBOL(ide_end_taskfile);

/*
- * try_to_flush_leftover_data() is invoked in response to a drive
- * unexpectedly having its DRQ_STAT bit set. As an alternative to
- * resetting the drive, this routine tries to clear the condition
- * by read a sector's worth of data from the drive. Of course,
- * this may not help if the drive is *waiting* for data from *us*.
- */
-void task_try_to_flush_leftover_data (ide_drive_t *drive)
-{
- int i = (drive->mult_count ? drive->mult_count : 1) * SECTOR_WORDS;
-
- if (drive->media != ide_disk)
- return;
- while (i > 0) {
- u32 buffer[16];
- unsigned int wcount = (i > 16) ? 16 : i;
- i -= wcount;
- taskfile_input_data(drive, buffer, wcount);
- }
-}
-
-EXPORT_SYMBOL(task_try_to_flush_leftover_data);
-
-/*
- * taskfile_error() takes action based on the error returned by the drive.
- */
-ide_startstop_t taskfile_error (ide_drive_t *drive, const char *msg, u8 stat)
-{
- ide_hwif_t *hwif;
- struct request *rq;
- u8 err;
-
- err = taskfile_dump_status(drive, msg, stat);
- if (drive == NULL || (rq = HWGROUP(drive)->rq) == NULL)
- return ide_stopped;
-
- hwif = HWIF(drive);
- /* retry only "normal" I/O: */
- if (rq->flags & REQ_DRIVE_TASKFILE) {
- rq->errors = 1;
- ide_end_taskfile(drive, stat, err);
- return ide_stopped;
- }
- if (stat & BUSY_STAT || ((stat & WRERR_STAT) && !drive->nowerr)) {
- /* other bits are useless when BUSY */
- rq->errors |= ERROR_RESET;
- } else {
- if (drive->media != ide_disk)
- goto media_out;
- if (stat & ERR_STAT) {
- /* err has different meaning on cdrom and tape */
- if (err == ABRT_ERR) {
- if (drive->select.b.lba &&
- (hwif->INB(IDE_COMMAND_REG) == WIN_SPECIFY))
- /* some newer drives don't
- * support WIN_SPECIFY
- */
- return ide_stopped;
- } else if ((err & BAD_CRC) == BAD_CRC) {
- /* UDMA crc error -- just retry the operation */
- drive->crc_count++;
- } else if (err & (BBD_ERR | ECC_ERR)) {
- /* retries won't help these */
- rq->errors = ERROR_MAX;
- } else if (err & TRK0_ERR) {
- /* help it find track zero */
- rq->errors |= ERROR_RECAL;
- }
- }
-media_out:
- if ((stat & DRQ_STAT) && rq_data_dir(rq) != WRITE)
- task_try_to_flush_leftover_data(drive);
- }
- if (hwif->INB(IDE_STATUS_REG) & (BUSY_STAT|DRQ_STAT)) {
- /* force an abort */
- hwif->OUTB(WIN_IDLEIMMEDIATE, IDE_COMMAND_REG);
- }
- if (rq->errors >= ERROR_MAX) {
- DRIVER(drive)->end_request(drive, 0, 0);
- } else {
- if ((rq->errors & ERROR_RESET) == ERROR_RESET) {
- ++rq->errors;
- return ide_do_reset(drive);
- }
- if ((rq->errors & ERROR_RECAL) == ERROR_RECAL)
- drive->special.b.recalibrate = 1;
- ++rq->errors;
- }
- return ide_stopped;
-}
-
-EXPORT_SYMBOL(taskfile_error);
-
-/*
* set_multmode_intr() is invoked on completion of a WIN_SETMULT cmd.
*/
ide_startstop_t set_multmode_intr (ide_drive_t *drive)
diff -uNr linux-2.5.66-ide-pio-4/drivers/ide/ide.c linux/drivers/ide/ide.c
--- linux-2.5.66-ide-pio-4/drivers/ide/ide.c Tue Mar 25 22:53:09 2003
+++ linux/drivers/ide/ide.c Thu Mar 27 22:38:42 2003
@@ -421,19 +421,20 @@
(long long) sectors,
high, low);
} else {
- u8 cur = hwif->INB(IDE_SELECT_REG);
- if (cur & 0x40) { /* using LBA? */
- printk(", LBAsect=%ld", (unsigned long)
- ((cur&0xf)<<24)
- |(hwif->INB(IDE_HCYL_REG)<<16)
- |(hwif->INB(IDE_LCYL_REG)<<8)
- | hwif->INB(IDE_SECTOR_REG));
+ u8 cur = hwif->INB(IDE_SELECT_REG);
+ u8 low = hwif->INB(IDE_LCYL_REG);
+ u8 high = hwif->INB(IDE_HCYL_REG);
+ u8 sect = hwif->INB(IDE_SECTOR_REG);
+ /* using LBA? */
+ if (cur & 0x40) {
+ printk(", LBAsect=%d", (u32)
+ ((cur&0xf)<<24)
+ |(high<<16)|(low<<8)
+ |sect);
} else {
printk(", CHS=%d/%d/%d",
- (hwif->INB(IDE_HCYL_REG)<<8) +
- hwif->INB(IDE_LCYL_REG),
- cur & 0xf,
- hwif->INB(IDE_SECTOR_REG));
+ ((high<<8) + low),
+ (cur & 0xf), sect);
}
}
if (HWGROUP(drive) && HWGROUP(drive)->rq)

Subject: [PATCH] new IDE PIO handlers 3/4


# Rewritten PIO handlers, both single and multiple sector sizes.
#
# They make use of new bio travelsing code and are supposed to be
# correct in respect to ATA state machine.
#
# Patch 3/4 - Remove old (now dead) PIO handlers from ide-taskfile.c.
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66-ide-pio-1/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.66-ide-pio-1/drivers/ide/ide-taskfile.c Wed Mar 26 22:49:34 2003
+++ linux/drivers/ide/ide-taskfile.c Wed Mar 26 22:52:45 2003
@@ -793,407 +793,6 @@

EXPORT_SYMBOL(pre_task_mulout_intr);

-
-#if 0
-/*
- * Handler for command with PIO data-in phase, READ
- */
-/*
- * FIXME before 2.4 enable ...
- * DATA integrity issue upon error. <[email protected]>
- */
-ide_startstop_t task_in_intr (ide_drive_t *drive)
-{
- struct request *rq = HWGROUP(drive)->rq;
- ide_hwif_t *hwif = HWIF(drive);
- char *pBuf = NULL;
- u8 stat;
- unsigned long flags;
-
- if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG),DATA_READY,BAD_R_STAT)) {
- if (stat & (ERR_STAT|DRQ_STAT)) {
-#if 0
- DTF("%s: attempting to recover last " \
- "sector counter status=0x%02x\n",
- drive->name, stat);
- /*
- * Expect a BUG BOMB if we attempt to rewind the
- * offset in the BH aka PAGE in the current BLOCK
- * segment. This is different than the HOST segment.
- */
-#endif
- if (!rq->bio)
- rq->current_nr_sectors++;
- return DRIVER(drive)->error(drive, "task_in_intr", stat);
- }
- if (!(stat & BUSY_STAT)) {
- DTF("task_in_intr to Soon wait for next interrupt\n");
- if (HWGROUP(drive)->handler == NULL)
- ide_set_handler(drive, &task_in_intr, WAIT_WORSTCASE, NULL);
- return ide_started;
- }
- }
-#if 0
-
- /*
- * Holding point for a brain dump of a thought :-/
- */
-
- if (!OK_STAT(stat,DRIVE_READY,drive->bad_wstat)) {
- DTF("%s: READ attempting to recover last " \
- "sector counter status=0x%02x\n",
- drive->name, stat);
- rq->current_nr_sectors++;
- return DRIVER(drive)->error(drive, "task_in_intr", stat);
- }
- if (!rq->current_nr_sectors)
- if (!DRIVER(drive)->end_request(drive, 1, 0))
- return ide_stopped;
-
- if (--rq->current_nr_sectors <= 0)
- if (!DRIVER(drive)->end_request(drive, 1, 0))
- return ide_stopped;
-#endif
-
- pBuf = task_map_rq(rq, &flags);
- DTF("Read: %p, rq->current_nr_sectors: %d, stat: %02x\n",
- pBuf, (int) rq->current_nr_sectors, stat);
- taskfile_input_data(drive, pBuf, SECTOR_WORDS);
- task_unmap_rq(rq, pBuf, &flags);
- /*
- * 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 <= 0)
- if (!DRIVER(drive)->end_request(drive, 1, 0))
- return ide_stopped;
- /*
- * ERM, it is techincally legal to leave/exit here but it makes
- * a mess of the code ...
- */
- if (HWGROUP(drive)->handler == NULL)
- ide_set_handler(drive, &task_in_intr, WAIT_WORSTCASE, NULL);
- return ide_started;
-}
-
-EXPORT_SYMBOL(task_in_intr);
-
-/*
- * Handler for command with Read Multiple
- */
-ide_startstop_t task_mulin_intr (ide_drive_t *drive)
-{
- ide_hwif_t *hwif = HWIF(drive);
- struct request *rq = HWGROUP(drive)->rq;
- char *pBuf = NULL;
- unsigned int msect = drive->mult_count;
- unsigned int nsect;
- unsigned long flags;
- u8 stat;
-
- if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG),DATA_READY,BAD_R_STAT)) {
- if (stat & (ERR_STAT|DRQ_STAT)) {
- if (!rq->bio) {
- rq->current_nr_sectors += drive->mult_count;
- /*
- * NOTE: could rewind beyond beginning :-/
- */
- } else {
- printk(KERN_ERR "%s: MULTI-READ assume all data " \
- "transfered is bad status=0x%02x\n",
- drive->name, stat);
- }
- return DRIVER(drive)->error(drive, "task_mulin_intr", stat);
- }
- /* no data yet, so wait for another interrupt */
- if (HWGROUP(drive)->handler == NULL)
- ide_set_handler(drive, &task_mulin_intr, WAIT_WORSTCASE, NULL);
- return ide_started;
- }
-
- do {
- nsect = rq->current_nr_sectors;
- if (nsect > msect)
- nsect = msect;
- pBuf = task_map_rq(rq, &flags);
- DTF("Multiread: %p, nsect: %d, msect: %d, " \
- " rq->current_nr_sectors: %d\n",
- pBuf, nsect, msect, rq->current_nr_sectors);
- 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
- * 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))
- return ide_stopped;
- }
- } while (msect);
- if (HWGROUP(drive)->handler == NULL)
- ide_set_handler(drive, &task_mulin_intr, WAIT_WORSTCASE, NULL);
- return ide_started;
-}
-
-EXPORT_SYMBOL(task_mulin_intr);
-
-/*
- * VERIFY ME before 2.4 ... unexpected race is possible based on details
- * RMK with 74LS245/373/374 TTL buffer logic because of passthrough.
- */
-ide_startstop_t pre_task_out_intr (ide_drive_t *drive, struct request *rq)
-{
- char *pBuf = NULL;
- unsigned long flags;
- ide_startstop_t startstop;
-
- if (ide_wait_stat(&startstop, drive, DATA_READY,
- drive->bad_wstat, WAIT_DRQ)) {
- printk(KERN_ERR "%s: no DRQ after issuing WRITE%s\n",
- drive->name,
- drive->addressing ? "_EXT" : "");
- return startstop;
- }
- /* For Write_sectors we need to stuff the first sector */
- pBuf = task_map_rq(rq, &flags);
- taskfile_output_data(drive, pBuf, SECTOR_WORDS);
- rq->current_nr_sectors--;
- task_unmap_rq(rq, pBuf, &flags);
- return ide_started;
-}
-
-EXPORT_SYMBOL(pre_task_out_intr);
-
-/*
- * Handler for command with PIO data-out phase WRITE
- *
- * WOOHOO this is a CORRECT STATE DIAGRAM NOW, <[email protected]>
- */
-ide_startstop_t task_out_intr (ide_drive_t *drive)
-{
- ide_hwif_t *hwif = HWIF(drive);
- struct request *rq = HWGROUP(drive)->rq;
- char *pBuf = NULL;
- unsigned long flags;
- u8 stat;
-
- if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG), DRIVE_READY, drive->bad_wstat)) {
- DTF("%s: WRITE attempting to recover last " \
- "sector counter status=0x%02x\n",
- drive->name, stat);
- rq->current_nr_sectors++;
- return DRIVER(drive)->error(drive, "task_out_intr", stat);
- }
- /*
- * 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 ((rq->current_nr_sectors==1) ^ (stat & DRQ_STAT)) {
- rq = HWGROUP(drive)->rq;
- pBuf = task_map_rq(rq, &flags);
- DTF("write: %p, rq->current_nr_sectors: %d\n",
- pBuf, (int) rq->current_nr_sectors);
- taskfile_output_data(drive, pBuf, SECTOR_WORDS);
- task_unmap_rq(rq, pBuf, &flags);
- rq->errors = 0;
- rq->current_nr_sectors--;
- }
- if (HWGROUP(drive)->handler == NULL)
- ide_set_handler(drive, &task_out_intr, WAIT_WORSTCASE, NULL);
- return ide_started;
-}
-
-EXPORT_SYMBOL(task_out_intr);
-
-#undef ALTERNATE_STATE_DIAGRAM_MULTI_OUT
-
-ide_startstop_t pre_task_mulout_intr (ide_drive_t *drive, struct request *rq)
-{
-#ifdef ALTERNATE_STATE_DIAGRAM_MULTI_OUT
- ide_hwif_t *hwif = HWIF(drive);
- char *pBuf = NULL;
- unsigned int nsect = 0, msect = drive->mult_count;
- u8 stat;
- unsigned long flags;
-#endif /* ALTERNATE_STATE_DIAGRAM_MULTI_OUT */
-
- ide_task_t *args = rq->special;
- ide_startstop_t startstop;
-
-#if 0
- /*
- * assign private copy for multi-write
- */
- memcpy(&HWGROUP(drive)->wrq, rq, sizeof(struct request));
-#endif
-
- if (ide_wait_stat(&startstop, drive, DATA_READY,
- drive->bad_wstat, WAIT_DRQ)) {
- printk(KERN_ERR "%s: no DRQ after issuing %s\n",
- drive->name,
- drive->addressing ? "MULTWRITE_EXT" : "MULTWRITE");
- return startstop;
- }
-#ifdef ALTERNATE_STATE_DIAGRAM_MULTI_OUT
-
- do {
- nsect = rq->current_nr_sectors;
- if (nsect > msect)
- nsect = msect;
- pBuf = task_map_rq(rq, &flags);
- DTF("Pre-Multiwrite: %p, nsect: %d, msect: %d, " \
- "rq->current_nr_sectors: %ld\n",
- pBuf, nsect, msect, rq->current_nr_sectors);
- 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;
- }
- }
- } while (msect);
- rq->errors = 0;
- return ide_started;
-#else /* ! ALTERNATE_STATE_DIAGRAM_MULTI_OUT */
- if (!(drive_is_ready(drive))) {
- int i;
- for (i=0; i<100; i++) {
- if (drive_is_ready(drive))
- break;
- }
- }
-
- /*
- * WARNING :: if the drive as not acked good status we may not
- * move the DATA-TRANSFER T-Bar as BSY != 0. <[email protected]>
- */
- return args->handler(drive);
-#endif /* ALTERNATE_STATE_DIAGRAM_MULTI_OUT */
-}
-
-EXPORT_SYMBOL(pre_task_mulout_intr);
-
-/*
- * FIXME before enabling in 2.4 ... DATA integrity issue upon error.
- */
-/*
- * Handler for command write multiple
- * Called directly from execute_drive_cmd for the first bunch of sectors,
- * afterwards only by the ISR
- */
-ide_startstop_t task_mulout_intr (ide_drive_t *drive)
-{
- ide_hwif_t *hwif = HWIF(drive);
- u8 stat = hwif->INB(IDE_STATUS_REG);
- struct request *rq = HWGROUP(drive)->rq;
- char *pBuf = NULL;
- ide_startstop_t startstop = ide_stopped;
- unsigned int msect = drive->mult_count;
- unsigned int nsect;
- unsigned long flags;
-
- /*
- * (ks/hs): Handle last IRQ on multi-sector transfer,
- * occurs after all data was sent in this chunk
- */
- if (rq->current_nr_sectors == 0) {
- if (stat & (ERR_STAT|DRQ_STAT)) {
- if (!rq->bio) {
- rq->current_nr_sectors += drive->mult_count;
- /*
- * NOTE: could rewind beyond beginning :-/
- */
- } else {
- printk(KERN_ERR "%s: MULTI-WRITE assume all data " \
- "transfered is bad status=0x%02x\n",
- drive->name, stat);
- }
- return DRIVER(drive)->error(drive, "task_mulout_intr", stat);
- }
- if (!rq->bio)
- DRIVER(drive)->end_request(drive, 1, 0);
- return startstop;
- }
- /*
- * DON'T be lazy code the above and below togather !!!
- */
- if (!OK_STAT(stat,DATA_READY,BAD_R_STAT)) {
- if (stat & (ERR_STAT|DRQ_STAT)) {
- if (!rq->bio) {
- rq->current_nr_sectors += drive->mult_count;
- /*
- * NOTE: could rewind beyond beginning :-/
- */
- } else {
- printk("%s: MULTI-WRITE assume all data " \
- "transfered is bad status=0x%02x\n",
- drive->name, stat);
- }
- return DRIVER(drive)->error(drive, "task_mulout_intr", stat);
- }
- /* no data yet, so wait for another interrupt */
- if (HWGROUP(drive)->handler == NULL)
- ide_set_handler(drive, &task_mulout_intr, WAIT_WORSTCASE, NULL);
- return ide_started;
- }
-
-#ifndef ALTERNATE_STATE_DIAGRAM_MULTI_OUT
- if (HWGROUP(drive)->handler != NULL) {
- unsigned long lflags;
- spin_lock_irqsave(&ide_lock, lflags);
- HWGROUP(drive)->handler = NULL;
- del_timer(&HWGROUP(drive)->timer);
- spin_unlock_irqrestore(&ide_lock, lflags);
- }
-#endif /* ALTERNATE_STATE_DIAGRAM_MULTI_OUT */
-
- do {
- nsect = rq->current_nr_sectors;
- if (nsect > msect)
- nsect = msect;
- pBuf = task_map_rq(rq, &flags);
- DTF("Multiwrite: %p, nsect: %d, msect: %d, " \
- "rq->current_nr_sectors: %ld\n",
- pBuf, nsect, msect, rq->current_nr_sectors);
- 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;
- }
- } while (msect);
- rq->errors = 0;
- if (HWGROUP(drive)->handler == NULL)
- ide_set_handler(drive, &task_mulout_intr, WAIT_WORSTCASE, NULL);
- return ide_started;
-}
-
-EXPORT_SYMBOL(task_mulout_intr);
-#endif /* 0 */
-
/* Called by internal to feature out type of command being called */
//ide_pre_handler_t * ide_pre_handler_parser (task_struct_t *taskfile, hob_struct_t *hobfile)
ide_pre_handler_t * ide_pre_handler_parser (struct hd_drive_task_hdr *taskfile, struct hd_drive_hob_hdr *hobfile)

Subject: Re: [ANNOUNCE] 2.5.66 bio traversal + IDE PIO patches on the way


On 27 Mar 2003, Alan Cox wrote:

> On Thu, 2003-03-27 at 23:46, Bartlomiej Zolnierkiewicz wrote:
> > 2.5.66-ide-pio-1-A0.diff
> > 2.5.66-ide-pio-2-A0.diff
> > and turn on IDE_TASKFILE_IO config option in IDE menu
> >
> > As always with block or IDE changes special care is _strongly_
> > recommended, don't blame me if it eats your fs :-).
>
> The IDE taskfile stuff for I/O is known broken. Thats why it
> is currently disabled. I plan to keep it that way until 2.7

What is broken?
It works just as good as standard code
(with taskfile fixes from ide-pio-1 and ide-pio-2 patches)
...or I am missing something?

--
Bartlomiej Zolnierkiewicz

Subject: [PATCH] new IDE PIO handlers 4/4


# Rewritten PIO handlers, both single and multiple sector sizes.
#
# They make use of new bio travelsing code and are supposed to be
# correct in respect to ATA state machine.
#
# Patch 4/4 - Remove not taskfile PIO handlers from ide-disk.c.
# Also remove not taskfile do_rw_disk().
#
# Now only new PIO handlers and taskfile based do_rw_disk() are used.
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66-ide-pio-2/drivers/ide/ide-disk.c linux/drivers/ide/ide-disk.c
--- linux-2.5.66-ide-pio-2/drivers/ide/ide-disk.c Thu Mar 27 00:07:59 2003
+++ linux/drivers/ide/ide-disk.c Thu Mar 27 00:10:59 2003
@@ -145,427 +145,6 @@
return ret;
}

-#ifndef CONFIG_IDE_TASKFILE_IO
-
-static int driver_blocked;
-
-/*
- * read_intr() is the handler for disk read/multread interrupts
- */
-static ide_startstop_t read_intr (ide_drive_t *drive)
-{
- ide_hwif_t *hwif = HWIF(drive);
- u32 i = 0, nsect = 0, msect = drive->mult_count;
- struct request *rq;
- unsigned long flags;
- u8 stat;
- char *to;
-
- /* new way for dealing with premature shared PCI interrupts */
- if (!OK_STAT(stat=hwif->INB(IDE_STATUS_REG),DATA_READY,BAD_R_STAT)) {
- if (stat & (ERR_STAT|DRQ_STAT)) {
- return DRIVER(drive)->error(drive, "read_intr", stat);
- }
- /* no data yet, so wait for another interrupt */
- ide_set_handler(drive, &read_intr, WAIT_CMD, NULL);
- return ide_started;
- }
-
-read_next:
- rq = HWGROUP(drive)->rq;
- if (msect) {
- if ((nsect = rq->current_nr_sectors) > msect)
- nsect = msect;
- msect -= nsect;
- } else
- nsect = 1;
- to = ide_map_buffer(rq, &flags);
- taskfile_input_data(drive, to, nsect * SECTOR_WORDS);
-#ifdef DEBUG
- printk("%s: read: sectors(%ld-%ld), buffer=0x%08lx, remaining=%ld\n",
- drive->name, rq->sector, rq->sector+nsect-1,
- (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);
- /*
- * Another BH Page walker and DATA INTEGRITY 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;
- ide_set_handler(drive, &read_intr, WAIT_CMD, NULL);
- return ide_started;
- }
- return ide_stopped;
-}
-
-/*
- * write_intr() is the handler for disk write interrupts
- */
-static ide_startstop_t write_intr (ide_drive_t *drive)
-{
- ide_hwgroup_t *hwgroup = HWGROUP(drive);
- ide_hwif_t *hwif = HWIF(drive);
- struct request *rq = hwgroup->rq;
- u32 i = 0;
- u8 stat;
-
- if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG),
- DRIVE_READY, drive->bad_wstat)) {
- printk("%s: write_intr error1: nr_sectors=%ld, stat=0x%02x\n",
- drive->name, rq->nr_sectors, stat);
- } else {
-#ifdef DEBUG
- printk("%s: write: sector %ld, buffer=0x%08lx, remaining=%ld\n",
- drive->name, rq->sector, (unsigned long) rq->buffer,
- 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);
- ide_set_handler(drive, &write_intr, WAIT_CMD, NULL);
- return ide_started;
- }
- return ide_stopped;
- }
- /* the original code did this here (?) */
- return ide_stopped;
- }
- return DRIVER(drive)->error(drive, "write_intr", stat);
-}
-
-/*
- * ide_multwrite() transfers a block of up to mcount sectors of data
- * to a drive as part of a disk multiple-sector write operation.
- *
- * Returns 0 on success.
- *
- * Note that we may be called from two contexts - the do_rw_disk context
- * and IRQ context. The IRQ can happen any time after we've output the
- * full "mcount" number of sectors, so we must make sure we update the
- * state _before_ we output the final part of the data!
- *
- * The update and return to BH is a BLOCK Layer Fakey to get more data
- * to satisfy the hardware atomic segment. If the hardware atomic segment
- * is shorter or smaller than the BH segment then we should be OKAY.
- * This is only valid if we can rewind the rq->current_nr_sectors counter.
- */
-int ide_multwrite (ide_drive_t *drive, unsigned int mcount)
-{
- ide_hwgroup_t *hwgroup = HWGROUP(drive);
- struct request *rq = &hwgroup->wrq;
-
- do {
- char *buffer;
- int nsect = rq->current_nr_sectors;
- unsigned long flags;
-
- if (nsect > mcount)
- nsect = mcount;
- mcount -= nsect;
- buffer = ide_map_buffer(rq, &flags);
-
- rq->sector += nsect;
- rq->nr_sectors -= nsect;
- rq->current_nr_sectors -= nsect;
-
- /* Do we move to the next bh after this? */
- if (!rq->current_nr_sectors) {
- struct bio *bio = rq->bio;
-
- /*
- * only move to next bio, when we have processed
- * all bvecs in this one.
- */
- if (++bio->bi_idx >= bio->bi_vcnt) {
- bio->bi_idx = 0;
- bio = bio->bi_next;
- }
-
- /* end early early we ran out of requests */
- if (!bio) {
- mcount = 0;
- } else {
- rq->bio = bio;
- rq->current_nr_sectors = bio_iovec(bio)->bv_len >> 9;
- rq->hard_cur_sectors = rq->current_nr_sectors;
- }
- }
-
- /*
- * Ok, we're all setup for the interrupt
- * re-entering us on the last transfer.
- */
- taskfile_output_data(drive, buffer, nsect<<7);
- ide_unmap_buffer(rq, buffer, &flags);
- } while (mcount);
-
- return 0;
-}
-
-/*
- * multwrite_intr() is the handler for disk multwrite interrupts
- */
-static ide_startstop_t multwrite_intr (ide_drive_t *drive)
-{
- ide_hwgroup_t *hwgroup = HWGROUP(drive);
- ide_hwif_t *hwif = HWIF(drive);
- struct request *rq = &hwgroup->wrq;
- u8 stat;
-
- stat = hwif->INB(IDE_STATUS_REG);
- if (OK_STAT(stat, DRIVE_READY, drive->bad_wstat)) {
- if (stat & DRQ_STAT) {
- /*
- * The drive wants data. Remember rq is the copy
- * of the request
- */
- if (rq->nr_sectors) {
- if (ide_multwrite(drive, drive->mult_count))
- return ide_stopped;
- ide_set_handler(drive, &multwrite_intr, WAIT_CMD, NULL);
- return ide_started;
- }
- } else {
- /*
- * If the copy has all the blocks completed then
- * we can end the original request.
- */
- if (!rq->nr_sectors) { /* all done? */
- rq = hwgroup->rq;
- ide_end_request(drive, 1, rq->nr_sectors);
- return ide_stopped;
- }
- }
- /* the original code did this here (?) */
- return ide_stopped;
- }
- return DRIVER(drive)->error(drive, "multwrite_intr", stat);
-}
-
-/*
- * do_rw_disk() issues READ and WRITE commands to a disk,
- * using LBA if supported, or CHS otherwise, to address sectors.
- * It also takes care of issuing special DRIVE_CMDs.
- */
-static ide_startstop_t do_rw_disk (ide_drive_t *drive, struct request *rq, sector_t block)
-{
- ide_hwif_t *hwif = HWIF(drive);
- u8 lba48 = (drive->addressing == 1) ? 1 : 0;
- task_ioreg_t command = WIN_NOP;
- ata_nsector_t nsectors;
-
- nsectors.all = (u16) rq->nr_sectors;
-
- if (driver_blocked)
- panic("Request while ide driver is blocked?");
-
-#if defined(CONFIG_BLK_DEV_PDC4030) || defined(CONFIG_BLK_DEV_PDC4030_MODULE)
- if (IS_PDC4030_DRIVE)
- return promise_rw_disk(drive, rq, block);
-#endif /* CONFIG_BLK_DEV_PDC4030 */
-
- if (drive->using_tcq && idedisk_start_tag(drive, rq)) {
- if (!ata_pending_commands(drive))
- BUG();
-
- return ide_started;
- }
-
- if (IDE_CONTROL_REG)
- hwif->OUTB(drive->ctl, IDE_CONTROL_REG);
-
- if (drive->select.b.lba) {
- if (drive->addressing == 1) {
- task_ioreg_t tasklets[10];
-
- if (blk_rq_tagged(rq)) {
- tasklets[0] = nsectors.b.low;
- tasklets[1] = nsectors.b.high;
- tasklets[2] = rq->tag << 3;
- tasklets[3] = 0;
- } else {
- tasklets[0] = 0;
- tasklets[1] = 0;
- tasklets[2] = nsectors.b.low;
- tasklets[3] = nsectors.b.high;
- }
-
- tasklets[4] = (task_ioreg_t) block;
- tasklets[5] = (task_ioreg_t) (block>>8);
- tasklets[6] = (task_ioreg_t) (block>>16);
- tasklets[7] = (task_ioreg_t) (block>>24);
- if (sizeof(block) == 4) {
- tasklets[8] = (task_ioreg_t) 0;
- tasklets[9] = (task_ioreg_t) 0;
- } else {
- tasklets[8] = (task_ioreg_t)((u64)block >> 32);
- tasklets[9] = (task_ioreg_t)((u64)block >> 40);
- }
-#ifdef DEBUG
- printk("%s: %sing: LBAsect=%lu, sectors=%ld, "
- "buffer=0x%08lx, LBAsect=0x%012lx\n",
- drive->name,
- rq_data_dir(rq)==READ?"read":"writ",
- block,
- rq->nr_sectors,
- (unsigned long) rq->buffer,
- block);
- printk("%s: 0x%02x%02x 0x%02x%02x%02x%02x%02x%02x\n",
- drive->name, tasklets[3], tasklets[2],
- tasklets[9], tasklets[8], tasklets[7],
- tasklets[6], tasklets[5], tasklets[4]);
-#endif
- hwif->OUTB(tasklets[1], IDE_FEATURE_REG);
- hwif->OUTB(tasklets[3], IDE_NSECTOR_REG);
- hwif->OUTB(tasklets[7], IDE_SECTOR_REG);
- hwif->OUTB(tasklets[8], IDE_LCYL_REG);
- hwif->OUTB(tasklets[9], IDE_HCYL_REG);
-
- hwif->OUTB(tasklets[0], IDE_FEATURE_REG);
- hwif->OUTB(tasklets[2], IDE_NSECTOR_REG);
- hwif->OUTB(tasklets[4], IDE_SECTOR_REG);
- hwif->OUTB(tasklets[5], IDE_LCYL_REG);
- hwif->OUTB(tasklets[6], IDE_HCYL_REG);
- hwif->OUTB(0x00|drive->select.all,IDE_SELECT_REG);
- } else {
-#ifdef DEBUG
- printk("%s: %sing: LBAsect=%llu, sectors=%ld, "
- "buffer=0x%08lx\n",
- drive->name,
- rq_data_dir(rq)==READ?"read":"writ",
- (unsigned long long)block, rq->nr_sectors,
- (unsigned long) rq->buffer);
-#endif
- if (blk_rq_tagged(rq)) {
- hwif->OUTB(nsectors.b.low, IDE_FEATURE_REG);
- hwif->OUTB(rq->tag << 3, IDE_NSECTOR_REG);
- } else {
- hwif->OUTB(0x00, IDE_FEATURE_REG);
- hwif->OUTB(nsectors.b.low, IDE_NSECTOR_REG);
- }
-
- hwif->OUTB(block, IDE_SECTOR_REG);
- hwif->OUTB(block>>=8, IDE_LCYL_REG);
- hwif->OUTB(block>>=8, IDE_HCYL_REG);
- hwif->OUTB(((block>>8)&0x0f)|drive->select.all,IDE_SELECT_REG);
- }
- } else {
- unsigned int sect,head,cyl,track;
- track = (int)block / drive->sect;
- sect = (int)block % drive->sect + 1;
- hwif->OUTB(sect, IDE_SECTOR_REG);
- head = track % drive->head;
- cyl = track / drive->head;
-
- if (blk_rq_tagged(rq)) {
- hwif->OUTB(nsectors.b.low, IDE_FEATURE_REG);
- hwif->OUTB(rq->tag << 3, IDE_NSECTOR_REG);
- } else {
- hwif->OUTB(0x00, IDE_FEATURE_REG);
- hwif->OUTB(nsectors.b.low, IDE_NSECTOR_REG);
- }
-
- hwif->OUTB(cyl, IDE_LCYL_REG);
- hwif->OUTB(cyl>>8, IDE_HCYL_REG);
- hwif->OUTB(head|drive->select.all,IDE_SELECT_REG);
-#ifdef DEBUG
- printk("%s: %sing: CHS=%d/%d/%d, sectors=%ld, buffer=0x%08lx\n",
- drive->name, rq_data_dir(rq)==READ?"read":"writ", cyl,
- head, sect, rq->nr_sectors, (unsigned long) rq->buffer);
-#endif
- }
-
- if (rq_data_dir(rq) == READ) {
- if (blk_rq_tagged(rq))
- return hwif->ide_dma_queued_read(drive);
-
- if (drive->using_dma && !hwif->ide_dma_read(drive))
- return ide_started;
-
- command = ((drive->mult_count) ?
- ((lba48) ? WIN_MULTREAD_EXT : WIN_MULTREAD) :
- ((lba48) ? WIN_READ_EXT : WIN_READ));
- ide_execute_command(drive, command, &read_intr, WAIT_CMD, NULL);
- return ide_started;
- } else if (rq_data_dir(rq) == WRITE) {
- ide_startstop_t startstop;
-
- if (blk_rq_tagged(rq))
- return hwif->ide_dma_queued_write(drive);
-
- if (drive->using_dma && !(HWIF(drive)->ide_dma_write(drive)))
- return ide_started;
-
- command = ((drive->mult_count) ?
- ((lba48) ? WIN_MULTWRITE_EXT : WIN_MULTWRITE) :
- ((lba48) ? WIN_WRITE_EXT : WIN_WRITE));
- hwif->OUTB(command, IDE_COMMAND_REG);
-
- if (ide_wait_stat(&startstop, drive, DATA_READY,
- drive->bad_wstat, WAIT_DRQ)) {
- printk(KERN_ERR "%s: no DRQ after issuing %s\n",
- drive->name,
- drive->mult_count ? "MULTWRITE" : "WRITE");
- return startstop;
- }
- if (!drive->unmask)
- local_irq_disable();
- if (drive->mult_count) {
- ide_hwgroup_t *hwgroup = HWGROUP(drive);
- /*
- * Ugh.. this part looks ugly because we MUST set up
- * the interrupt handler before outputting the first block
- * of data to be written. If we hit an error (corrupted buffer list)
- * in ide_multwrite(), then we need to remove the handler/timer
- * before returning. Fortunately, this NEVER happens (right?).
- *
- * Except when you get an error it seems...
- *
- * MAJOR DATA INTEGRITY BUG !!! only if we error
- */
- hwgroup->wrq = *rq; /* scratchpad */
- ide_set_handler(drive, &multwrite_intr, WAIT_CMD, NULL);
- if (ide_multwrite(drive, drive->mult_count)) {
- unsigned long flags;
- spin_lock_irqsave(&ide_lock, flags);
- hwgroup->handler = NULL;
- del_timer(&hwgroup->timer);
- spin_unlock_irqrestore(&ide_lock, flags);
- return ide_stopped;
- }
- } else {
- unsigned long flags;
- char *to = ide_map_buffer(rq, &flags);
- ide_set_handler(drive, &write_intr, WAIT_CMD, NULL);
- taskfile_output_data(drive, to, SECTOR_WORDS);
- ide_unmap_buffer(rq, to, &flags);
- }
- return ide_started;
- }
- blk_dump_rq_flags(rq, "do_rw_disk - bad command");
- ide_end_request(drive, 0, 0);
- return ide_stopped;
-}
-
-#else /* CONFIG_IDE_TASKFILE_IO */
-
static ide_startstop_t chs_rw_disk(ide_drive_t *, struct request *, unsigned long);
static ide_startstop_t lba_28_rw_disk(ide_drive_t *, struct request *, unsigned long);
static ide_startstop_t lba_48_rw_disk(ide_drive_t *, struct request *, unsigned long long);
@@ -767,8 +346,6 @@
return do_rw_taskfile(drive, &args);
}

-#endif /* CONFIG_IDE_TASKFILE_IO */
-
static int do_idedisk_flushcache(ide_drive_t *drive);

static u8 idedisk_dump_status (ide_drive_t *drive, const char *msg, u8 stat)

2003-03-27 23:46:08

by Alan

[permalink] [raw]
Subject: Re: [ANNOUNCE] 2.5.66 bio traversal + IDE PIO patches on the way

On Thu, 2003-03-27 at 23:46, Bartlomiej Zolnierkiewicz wrote:
> 2.5.66-ide-pio-1-A0.diff
> 2.5.66-ide-pio-2-A0.diff
> and turn on IDE_TASKFILE_IO config option in IDE menu
>
> As always with block or IDE changes special care is _strongly_
> recommended, don't blame me if it eats your fs :-).

The IDE taskfile stuff for I/O is known broken. Thats why it
is currently disabled. I plan to keep it that way until 2.7


Subject: [PATCH] new IDE PIO handlers 2/4


# Rewritten PIO handlers, both single and multiple sector sizes.
#
# They make use of new bio travelsing code and are supposed to be
# correct in respect to ATA state machine.
#
# Patch 2/4 - Fix compilation of CONFIG_IDE_TASKFILE_IO.
# Fix read/write DMA commands handling in do_rw_taskfile()
# (b/c default return value is ide_stopped).
#
# New PIO handlers will be used if you turn on IDE_TASKFILE_IO config option.
#
# Bartlomiej Zolnierkiewicz <[email protected]>

diff -uNr linux-2.5.66-ide-pio-1/drivers/ide/Kconfig linux/drivers/ide/Kconfig
--- linux-2.5.66-ide-pio-1/drivers/ide/Kconfig Tue Mar 25 22:53:09 2003
+++ linux/drivers/ide/Kconfig Wed Mar 26 23:59:07 2003
@@ -219,7 +219,10 @@

If you are unsure, say N here.

-#bool ' IDE Taskfile IO' CONFIG_IDE_TASKFILE_IO
+config IDE_TASKFILE_IO
+ bool "IDE Taskfile IO"
+ depends on BLK_DEV_IDE
+
comment "IDE chipset support/bugfixes"
depends on BLK_DEV_IDE

diff -uNr linux-2.5.66-ide-pio-1/drivers/ide/ide-disk.c linux/drivers/ide/ide-disk.c
--- linux-2.5.66-ide-pio-1/drivers/ide/ide-disk.c Tue Mar 25 22:53:09 2003
+++ linux/drivers/ide/ide-disk.c Wed Mar 26 23:53:44 2003
@@ -73,8 +73,6 @@

#include "legacy/pdc4030.h"

-static int driver_blocked;
-
static inline u32 idedisk_read_24 (ide_drive_t *drive)
{
u8 hcyl = HWIF(drive)->INB(IDE_HCYL_REG);
@@ -133,8 +131,24 @@
return 0; /* lba_capacity value may be bad */
}

+static int idedisk_start_tag(ide_drive_t *drive, struct request *rq)
+{
+ unsigned long flags;
+ int ret = 1;
+
+ spin_lock_irqsave(&ide_lock, flags);
+
+ if (ata_pending_commands(drive) < drive->queue_depth)
+ ret = blk_queue_start_tag(&drive->queue, rq);
+
+ spin_unlock_irqrestore(&ide_lock, flags);
+ return ret;
+}
+
#ifndef CONFIG_IDE_TASKFILE_IO

+static int driver_blocked;
+
/*
* read_intr() is the handler for disk read/multread interrupts
*/
@@ -345,20 +359,6 @@
return DRIVER(drive)->error(drive, "multwrite_intr", stat);
}

-static int idedisk_start_tag(ide_drive_t *drive, struct request *rq)
-{
- unsigned long flags;
- int ret = 1;
-
- spin_lock_irqsave(&ide_lock, flags);
-
- if (ata_pending_commands(drive) < drive->queue_depth)
- ret = blk_queue_start_tag(&drive->queue, rq);
-
- spin_unlock_irqrestore(&ide_lock, flags);
- return ret;
-}
-
/*
* do_rw_disk() issues READ and WRITE commands to a disk,
* using LBA if supported, or CHS otherwise, to address sectors.
@@ -745,7 +745,7 @@
args.tfRegister[IDE_FEATURE_OFFSET] = sectors;
args.tfRegister[IDE_NSECTOR_OFFSET] = rq->tag << 3;
args.hobRegister[IDE_FEATURE_OFFSET_HOB] = sectors >> 8;
- args.hobRegister[IDE_NSECT_OFFSET_HOB] = 0;
+ args.hobRegister[IDE_NSECTOR_OFFSET_HOB] = 0;
} else {
args.tfRegister[IDE_NSECTOR_OFFSET] = sectors;
args.hobRegister[IDE_NSECTOR_OFFSET_HOB] = sectors >> 8;
diff -uNr linux-2.5.66-ide-pio-1/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.66-ide-pio-1/drivers/ide/ide-taskfile.c Wed Mar 26 22:54:54 2003
+++ linux/drivers/ide/ide-taskfile.c Wed Mar 26 23:50:20 2003
@@ -193,15 +193,15 @@
case WIN_WRITEDMA_ONCE:
case WIN_WRITEDMA:
case WIN_WRITEDMA_EXT:
- if (hwif->ide_dma_write(drive))
- return ide_stopped;
+ if (!hwif->ide_dma_write(drive))
+ return ide_started;
break;
case WIN_READDMA_ONCE:
case WIN_READDMA:
case WIN_READDMA_EXT:
case WIN_IDENTIFY_DMA:
- if (hwif->ide_dma_read(drive))
- return ide_stopped;
+ if (!hwif->ide_dma_read(drive))
+ return ide_started;
break;
case WIN_READDMA_QUEUED:
case WIN_READDMA_QUEUED_EXT:

2003-03-28 00:16:30

by Alan

[permalink] [raw]
Subject: Re: [ANNOUNCE] 2.5.66 bio traversal + IDE PIO patches on the way

On Fri, 2003-03-28 at 00:10, Bartlomiej Zolnierkiewicz wrote:
> > The IDE taskfile stuff for I/O is known broken. Thats why it
> > is currently disabled. I plan to keep it that way until 2.7
>
> What is broken?
> It works just as good as standard code
> (with taskfile fixes from ide-pio-1 and ide-pio-2 patches)
> ...or I am missing something?

I hadn't realised you had rewritten all the PIO side of it when
I wrote that. Looks like the revised plan is "pure taskfile for
2.6 care of Bartlomiej"

2003-03-28 00:13:51

by Alan

[permalink] [raw]
Subject: Re: [PATCH] new IDE PIO handlers 4/4

On Thu, 2003-03-27 at 23:57, Bartlomiej Zolnierkiewicz wrote:
> # Rewritten PIO handlers, both single and multiple sector sizes.
> #
> # They make use of new bio travelsing code and are supposed to be
> # correct in respect to ATA state machine.
> #
> # Patch 4/4 - Remove not taskfile PIO handlers from ide-disk.c.
> # Also remove not taskfile do_rw_disk().
> #
> # Now only new PIO handlers and taskfile based do_rw_disk() are used.

Looks good once the new stuff is tested for a few releases

2003-03-28 00:10:31

by Alan

[permalink] [raw]
Subject: Re: [PATCH] IDE 2.5.66-masked_irq-fix-A0

On Thu, 2003-03-27 at 23:54, Bartlomiej Zolnierkiewicz wrote:
> # Revert previous masked_irq handling for ide_do_request().
> # Fixes "hda: lost interrupt" bug.

Rejected. Breaks hardware where someone put IDE on IRQ0, the
logic is sound enough and I will fix it properly using the NO_IRQ
stuff

2003-03-28 00:11:54

by Alan

[permalink] [raw]
Subject: Re: [PATCH] new IDE PIO handlers 2/4

On Thu, 2003-03-27 at 23:56, Bartlomiej Zolnierkiewicz wrote:
> # Rewritten PIO handlers, both single and multiple sector sizes.
> #
> # They make use of new bio travelsing code and are supposed to be
> # correct in respect to ATA state machine.

Very nice


Subject: Re: [ANNOUNCE] 2.5.66 bio traversal + IDE PIO patches on the way


On 28 Mar 2003, Alan Cox wrote:

> On Fri, 2003-03-28 at 00:10, Bartlomiej Zolnierkiewicz wrote:
> > > The IDE taskfile stuff for I/O is known broken. Thats why it
> > > is currently disabled. I plan to keep it that way until 2.7
> >
> > What is broken?
> > It works just as good as standard code
> > (with taskfile fixes from ide-pio-1 and ide-pio-2 patches)
> > ...or I am missing something?
>
> I hadn't realised you had rewritten all the PIO side of it when
> I wrote that. Looks like the revised plan is "pure taskfile for
> 2.6 care of Bartlomiej"

Okay, there is still plenty fixmes...

Subject: Re: [PATCH] IDE 2.5.66-masked_irq-fix-A0


On 28 Mar 2003, Alan Cox wrote:

> On Thu, 2003-03-27 at 23:54, Bartlomiej Zolnierkiewicz wrote:
> > # Revert previous masked_irq handling for ide_do_request().
> > # Fixes "hda: lost interrupt" bug.
>
> Rejected. Breaks hardware where someone put IDE on IRQ0, the

Yep.

> logic is sound enough and I will fix it properly using the NO_IRQ
> stuff

Something like that?
if (masked_irq != IDE_NO_IRQ && masked_irq != hwif->irq)

2003-03-28 03:09:13

by Andre Hedrick

[permalink] [raw]
Subject: Re: [PATCH] new IDE PIO handlers 1/4


I wondered when you would publish the BIO traversing code!
Now if we can make it generic for block then it will fix all the broken
scsi hba's that were lost in the transition, iirc.

Cheers,

On Fri, 28 Mar 2003, Bartlomiej Zolnierkiewicz wrote:

>
> # Rewritten PIO handlers, both single and multiple sector sizes.
> #
> # They make use of new bio travelsing code and are supposed to be
> # correct in respect to ATA state machine.
> #
> # Patch 1/4 - Implement them, do not activate yet.
> # Fix do_rw_taskfile() and flagged_taskfile() for correct
> # handling of new prehandlers.
> #
> # Bartlomiej Zolnierkiewicz <[email protected]>
>
> diff -uNr linux-2.5.66/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
> --- linux-2.5.66/drivers/ide/ide-taskfile.c Tue Mar 25 22:53:09 2003
> +++ linux/drivers/ide/ide-taskfile.c Wed Mar 26 22:49:34 2003
> @@ -5,6 +5,7 @@
> * Copyright (C) 2000-2002 Andre Hedrick <[email protected]>
> * Copyright (C) 2001-2002 Klaus Smolin
> * IBM Storage Technology Division
> + * Copyright (C) 2003 Bartlomiej Zolnierkiewicz
> *
> * The big the bad and the ugly.
> *
> @@ -176,9 +177,12 @@
>
> hwif->OUTB((taskfile->device_head & HIHI) | drive->select.all, IDE_SELECT_REG);
> if (task->handler != NULL) {
> - ide_execute_command(drive, taskfile->command, task->handler, WAIT_WORSTCASE, NULL);
> - if (task->prehandler != NULL)
> + if (task->prehandler != NULL) {
> + hwif->OUTBSYNC(drive, taskfile->command, IDE_COMMAND_REG);
> + ndelay(400); /* FIXME */
> return task->prehandler(drive, task->rq);
> + }
> + ide_execute_command(drive, taskfile->command, task->handler, WAIT_WORSTCASE, NULL);
> return ide_started;
> }
>
> @@ -535,6 +539,262 @@
>
> EXPORT_SYMBOL(task_no_data_intr);
>
> +#define PIO_IN 0
> +#define PIO_OUT 1
> +
> +static inline void task_sectors(ide_drive_t *drive, struct request *rq,
> + unsigned nsect, int rw)
> +{
> + unsigned long flags;
> + char *buf;
> +
> + buf = task_map_rq(rq, &flags);
> +
> + /*
> + * IRQ can happen instantly after reading/writing
> + * last sector of the datablock.
> + */
> + process_that_request_first(rq, nsect);
> +
> + if (rw == PIO_OUT)
> + taskfile_output_data(drive, buf, nsect * SECTOR_WORDS);
> + else
> + taskfile_input_data(drive, buf, nsect * SECTOR_WORDS);
> +
> + task_unmap_rq(rq, buf, &flags);
> +}
> +
> +/*
> + * Handler for command with PIO data-in phase (Read).
> + */
> +ide_startstop_t task_in_intr (ide_drive_t *drive)
> +{
> + struct request *rq = HWGROUP(drive)->rq;
> + u8 stat, good_stat;
> +
> + good_stat = DATA_READY;
> +check_status:
> + stat = HWIF(drive)->INB(IDE_STATUS_REG);
> + if (!OK_STAT(stat, good_stat, BAD_R_STAT)) {
> + if (stat & (ERR_STAT|DRQ_STAT))
> + return DRIVER(drive)->error(drive, __FUNCTION__, stat);
> + /* No data yet, so wait for another IRQ. */
> + ide_set_handler(drive, &task_in_intr, WAIT_WORSTCASE, NULL);
> + return ide_started;
> + }
> +
> + /*
> + * Complete previously submitted bios (if any).
> + * Status was already verifyied.
> + */
> + while (rq->hard_bio != rq->bio)
> + if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
> + return ide_stopped;
> +
> + rq->errors = 0;
> + task_sectors(drive, rq, 1, PIO_IN);
> +
> + /* If it was the last datablock check status and finish transfer. */
> + if (!rq->nr_sectors) {
> + good_stat = 0;
> + goto check_status;
> + }
> +
> + /* Still data left to transfer. */
> + ide_set_handler(drive, &task_in_intr, WAIT_WORSTCASE, NULL);
> +
> + return ide_started;
> +}
> +
> +EXPORT_SYMBOL(task_in_intr);
> +
> +/*
> + * Handler for command with PIO data-in phase (Read Multiple).
> + */
> +ide_startstop_t task_mulin_intr (ide_drive_t *drive)
> +{
> + struct request *rq = HWGROUP(drive)->rq;
> + unsigned int msect = drive->mult_count;
> + unsigned int nsect;
> + u8 stat, good_stat;
> +
> + good_stat = DATA_READY;
> +check_status:
> + stat = HWIF(drive)->INB(IDE_STATUS_REG);
> + if (!OK_STAT(stat, good_stat, BAD_R_STAT)) {
> + if (stat & (ERR_STAT|DRQ_STAT))
> + return DRIVER(drive)->error(drive, __FUNCTION__, stat);
> + /* No data yet, so wait for another IRQ. */
> + ide_set_handler(drive, &task_mulin_intr, WAIT_WORSTCASE, NULL);
> + return ide_started;
> + }
> +
> + /*
> + * Complete previously submitted bios (if any).
> + * Status was already verifyied.
> + */
> + while (rq->hard_bio != rq->bio)
> + if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
> + return ide_stopped;
> +
> + rq->errors = 0;
> + do {
> + nsect = rq->current_nr_sectors;
> + if (nsect > msect)
> + nsect = msect;
> +
> + task_sectors(drive, rq, nsect, PIO_IN);
> +
> + if (!rq->nr_sectors)
> + msect = 0;
> + else
> + msect -= nsect;
> + } while (msect);
> +
> + /* If it was the last datablock check status and finish transfer. */
> + if (!rq->nr_sectors) {
> + good_stat = 0;
> + goto check_status;
> + }
> +
> + /* Still data left to transfer. */
> + ide_set_handler(drive, &task_mulin_intr, WAIT_WORSTCASE, NULL);
> +
> + return ide_started;
> +}
> +
> +EXPORT_SYMBOL(task_mulin_intr);
> +
> +/*
> + * Handler for command with PIO data-out phase (Write).
> + */
> +ide_startstop_t task_out_intr (ide_drive_t *drive)
> +{
> + struct request *rq = HWGROUP(drive)->rq;
> + u8 stat;
> + int ok_stat;
> +
> + stat = HWIF(drive)->INB(IDE_STATUS_REG);
> + ok_stat = OK_STAT(stat, DRIVE_READY, drive->bad_wstat);
> +
> + if (!ok_stat || !rq->nr_sectors) {
> + if (stat & (ERR_STAT|DRQ_STAT))
> + return DRIVER(drive)->error(drive, __FUNCTION__, stat);
> + if (stat & BUSY_STAT) {
> + /* Not ready yet, so wait for another IRQ. */
> + ide_set_handler(drive, &task_out_intr, WAIT_WORSTCASE, NULL);
> + return ide_started;
> + }
> + }
> +
> + /*
> + * Complete previously submitted bios (if any).
> + * Status was already verifyied.
> + */
> + while (rq->hard_bio != rq->bio)
> + if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
> + return ide_stopped;
> +
> + /* Still data left to transfer. */
> + ide_set_handler(drive, &task_out_intr, WAIT_WORSTCASE, NULL);
> +
> + rq->errors = 0;
> + task_sectors(drive, rq, 1, PIO_OUT);
> +
> + return ide_started;
> +}
> +
> +EXPORT_SYMBOL(task_out_intr);
> +
> +ide_startstop_t pre_task_out_intr (ide_drive_t *drive, struct request *rq)
> +{
> + ide_startstop_t startstop;
> +
> + if (ide_wait_stat(&startstop, drive, DATA_READY,
> + drive->bad_wstat, WAIT_DRQ)) {
> + printk(KERN_ERR "%s: no DRQ after issuing WRITE%s\n",
> + drive->name, drive->addressing ? "_EXT" : "");
> + return startstop;
> + }
> +
> + return task_out_intr(drive);
> +}
> +
> +EXPORT_SYMBOL(pre_task_out_intr);
> +
> +/*
> + * Handler for command with PIO data-out phase (Write Multiple).
> + */
> +ide_startstop_t task_mulout_intr (ide_drive_t *drive)
> +{
> + struct request *rq = HWGROUP(drive)->rq;
> + unsigned int msect = drive->mult_count;
> + unsigned int nsect;
> + u8 stat;
> + int ok_stat;
> +
> + stat = HWIF(drive)->INB(IDE_STATUS_REG);
> + ok_stat = OK_STAT(stat, DRIVE_READY, drive->bad_wstat);
> +
> + if (!ok_stat || !rq->nr_sectors) {
> + if (stat & (ERR_STAT|DRQ_STAT))
> + return DRIVER(drive)->error(drive, __FUNCTION__, stat);
> + if (stat & BUSY_STAT) {
> + /* Not ready yet, so wait for another IRQ. */
> + ide_set_handler(drive, &task_mulout_intr, WAIT_WORSTCASE, NULL);
> + return ide_started;
> + }
> + }
> +
> + /*
> + * Complete previously submitted bios (if any).
> + * Status was already verifyied.
> + */
> + while (rq->hard_bio != rq->bio)
> + if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
> + return ide_stopped;
> +
> + /* Still data left to transfer. */
> + ide_set_handler(drive, &task_mulout_intr, WAIT_WORSTCASE, NULL);
> +
> + rq->errors = 0;
> + do {
> + nsect = rq->current_nr_sectors;
> + if (nsect > msect)
> + nsect = msect;
> +
> + task_sectors(drive, rq, nsect, PIO_OUT);
> +
> + if (!rq->nr_sectors)
> + msect = 0;
> + else
> + msect -= nsect;
> +
> + } while (msect);
> +
> + return ide_started;
> +}
> +
> +EXPORT_SYMBOL(task_mulout_intr);
> +
> +ide_startstop_t pre_task_mulout_intr (ide_drive_t *drive, struct request *rq)
> +{
> + ide_startstop_t startstop;
> +
> + if (ide_wait_stat(&startstop, drive, DATA_READY,
> + drive->bad_wstat, WAIT_DRQ)) {
> + printk(KERN_ERR "%s: no DRQ after issuing MULTWRITE%s\n",
> + drive->name, drive->addressing ? "_EXT" : "");
> + return startstop;
> + }
> +
> + return task_mulout_intr(drive);
> +}
> +
> +EXPORT_SYMBOL(pre_task_mulout_intr);
> +
> +
> +#if 0
> /*
> * Handler for command with PIO data-in phase, READ
> */
> @@ -932,6 +1192,7 @@
> }
>
> EXPORT_SYMBOL(task_mulout_intr);
> +#endif /* 0 */
>
> /* Called by internal to feature out type of command being called */
> //ide_pre_handler_t * ide_pre_handler_parser (task_struct_t *taskfile, hob_struct_t *hobfile)
> @@ -1873,10 +2134,14 @@
> if (task->handler == NULL)
> return ide_stopped;
>
> + if (task->prehandler != NULL) {
> + hwif->OUTBSYNC(drive, taskfile->command, IDE_COMMAND_REG);
> + ndelay(400); /* FIXME */
> + return task->prehandler(drive, HWGROUP(drive)->rq);
> + }
> +
> /* Issue the command */
> ide_execute_command(drive, taskfile->command, task->handler, WAIT_WORSTCASE, NULL);
> - if (task->prehandler != NULL)
> - return task->prehandler(drive, HWGROUP(drive)->rq);
> }
>
> return ide_started;
>
>

Andre Hedrick
LAD Storage Consulting Group

2003-03-28 03:11:41

by Andre Hedrick

[permalink] [raw]
Subject: Re: [ANNOUNCE] 2.5.66 bio traversal + IDE PIO patches on the way


Alan, use it -- it works.

Bart and I argued and debated the model back when MD was around.
I spent time explain the issues in the state machine and Bart got it.
Then I spent time with him and Suparna to work out the BIO issues.
Jens chimed in with the need changes to address our needs.

Try it before you wank on it!

On 27 Mar 2003, Alan Cox wrote:

> On Thu, 2003-03-27 at 23:46, Bartlomiej Zolnierkiewicz wrote:
> > 2.5.66-ide-pio-1-A0.diff
> > 2.5.66-ide-pio-2-A0.diff
> > and turn on IDE_TASKFILE_IO config option in IDE menu
> >
> > As always with block or IDE changes special care is _strongly_
> > recommended, don't blame me if it eats your fs :-).
>
> The IDE taskfile stuff for I/O is known broken. Thats why it
> is currently disabled. I plan to keep it that way until 2.7
>
>

Andre Hedrick
LAD Storage Consulting Group

2003-03-28 03:14:30

by Andre Hedrick

[permalink] [raw]
Subject: Re: [ANNOUNCE] 2.5.66 bio traversal + IDE PIO patches on the way


Yes it is revised and mapped into BIO proper.
This is one of the difficult things in 2.4 w/ BH but 2.5/2.6 BIO is
capable of satisfying the the state machine requirements proper.

Cheers,

On 28 Mar 2003, Alan Cox wrote:

> On Fri, 2003-03-28 at 00:10, Bartlomiej Zolnierkiewicz wrote:
> > > The IDE taskfile stuff for I/O is known broken. Thats why it
> > > is currently disabled. I plan to keep it that way until 2.7
> >
> > What is broken?
> > It works just as good as standard code
> > (with taskfile fixes from ide-pio-1 and ide-pio-2 patches)
> > ...or I am missing something?
>
> I hadn't realised you had rewritten all the PIO side of it when
> I wrote that. Looks like the revised plan is "pure taskfile for
> 2.6 care of Bartlomiej"
>
> -
> 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/
>

Andre Hedrick
LAD Storage Consulting Group

2003-03-28 17:13:21

by Alan

[permalink] [raw]
Subject: Re: [PATCH] IDE 2.5.66-masked_irq-fix-A0

On Fri, 2003-03-28 at 01:05, Bartlomiej Zolnierkiewicz wrote:
> > logic is sound enough and I will fix it properly using the NO_IRQ
> > stuff
>
> Something like that?
> if (masked_irq != IDE_NO_IRQ && masked_irq != hwif->irq)

I need to look at that further. Re-reading the code the non backed out logic
seems right. Its causing problems but I'm now less convinced it is the
guilty party.

If the hwif irq is not masked
mask it