Re: Scsi problems in 2.5.1-pre9

[Jens Axboe <axboe@suse.de>]

On Tue, Dec 11 2001, Jens Axboe wrote:
> On Tue, Dec 11 2001, Paul Larson wrote:
> > My hardware is a dual proc PII-300.  I was running LTP runalltests.sh
> > and it was on one of the growfiles tests when this problem occurred. 
> > The test hung, and I couldn't telnet into the machine or login to it,
> > but I could switch between VC's.  On the console, I had screenfulls of
> > errors like this:
> > 
> > Incorrect number of segments after building list
> > counted 11, received 7
> 
> Attached patch should fix it.

Aghr, here it is. Linus, please apply to 2.5.1-pre9.

(and yes note how the differences between the stock and scsi merge
functons are getting smaller -- in fact, we can complete merge the two
now)

diff -ur -X exclude /opt/kernel/linux-2.5.1-pre9/drivers/block/ll_rw_blk.c linux/drivers/block/ll_rw_blk.c
--- /opt/kernel/linux-2.5.1-pre9/drivers/block/ll_rw_blk.c	Tue Dec 11 05:01:50 2001
+++ linux/drivers/block/ll_rw_blk.c	Tue Dec 11 11:48:43 2001
@@ -358,6 +358,8 @@
 
 	if (!BIO_CONTIG(bio, nxt))
 		return 0;
+	if (bio->bi_size + nxt->bi_size > q->max_segment_size)
+		return 0;
 
 	/*
 	 * bio and nxt are contigous in memory, check if the queue allows
@@ -429,8 +431,10 @@
  * specific ones if so desired
  */
 static inline int ll_new_segment(request_queue_t *q, struct request *req,
-				 struct bio *bio, int nr_segs)
+				 struct bio *bio)
 {
+	int nr_segs = bio_hw_segments(q, bio);
+
 	if (req->nr_segments + nr_segs <= q->max_segments) {
 		req->nr_segments += nr_segs;
 		return 1;
@@ -443,41 +447,23 @@
 static int ll_back_merge_fn(request_queue_t *q, struct request *req, 
 			    struct bio *bio)
 {
-	int bio_segs;
-
 	if (req->nr_sectors + bio_sectors(bio) > q->max_sectors) {
 		req->flags |= REQ_NOMERGE;
 		return 0;
 	}
 
-	bio_segs = bio_hw_segments(q, bio);
-	if (blk_contig_segment(q, req->biotail, bio))
-		bio_segs--;
-
-	if (!bio_segs)
-		return 1;
-
-	return ll_new_segment(q, req, bio, bio_segs);
+	return ll_new_segment(q, req, bio);
 }
 
 static int ll_front_merge_fn(request_queue_t *q, struct request *req, 
 			     struct bio *bio)
 {
-	int bio_segs;
-
 	if (req->nr_sectors + bio_sectors(bio) > q->max_sectors) {
 		req->flags |= REQ_NOMERGE;
 		return 0;
 	}
 
-	bio_segs = bio_hw_segments(q, bio);
-	if (blk_contig_segment(q, bio, req->bio))
-		bio_segs--;
-
-	if (!bio_segs)
-		return 1;
-
-	return ll_new_segment(q, req, bio, bio_segs);
+	return ll_new_segment(q, req, bio);
 }
 
 static int ll_merge_requests_fn(request_queue_t *q, struct request *req,
@@ -1235,11 +1221,6 @@
 			break;
 		}
 
-		/*
-		 * this needs to be handled by q->make_request_fn, to just
-		 * setup a part of the bio in the request to enable easy
-		 * multiple passing
-		 */
 		BUG_ON(bio_sectors(bio) > q->max_sectors);
 
 		/*
@@ -1497,6 +1478,7 @@
 	while ((bio = req->bio)) {
 		nsect = bio_iovec(bio)->bv_len >> 9;
 
+		BIO_BUG_ON(bio_iovec(bio)->bv_len > bio->bi_size);
 
 		/*
 		 * not a complete bvec done
@@ -1515,11 +1497,12 @@
 		 * account transfer
 		 */
 		bio->bi_size -= bio_iovec(bio)->bv_len;
+		bio->bi_idx++;
 
 		nr_sectors -= nsect;
 		total_nsect += nsect;
 
-		if (++bio->bi_idx >= bio->bi_vcnt) {
+		if (!bio->bi_size) {
 			req->bio = bio->bi_next;
 
 			if (unlikely(bio_endio(bio, uptodate, total_nsect)))
@@ -1619,7 +1602,9 @@
 EXPORT_SYMBOL(blk_queue_max_segments);
 EXPORT_SYMBOL(blk_queue_max_segment_size);
 EXPORT_SYMBOL(blk_queue_hardsect_size);
+EXPORT_SYMBOL(blk_queue_segment_boundary);
 EXPORT_SYMBOL(blk_rq_map_sg);
 EXPORT_SYMBOL(blk_nohighio);
 EXPORT_SYMBOL(blk_dump_rq_flags);
 EXPORT_SYMBOL(submit_bio);
+EXPORT_SYMBOL(blk_contig_segment);
diff -ur -X exclude /opt/kernel/linux-2.5.1-pre9/drivers/scsi/ide-scsi.c linux/drivers/scsi/ide-scsi.c
--- /opt/kernel/linux-2.5.1-pre9/drivers/scsi/ide-scsi.c	Tue Dec 11 05:01:51 2001
+++ linux/drivers/scsi/ide-scsi.c	Tue Dec 11 09:24:14 2001
@@ -261,7 +261,7 @@
 	ide_drive_t *drive = hwgroup->drive;
 	idescsi_scsi_t *scsi = drive->driver_data;
 	struct request *rq = hwgroup->rq;
-	idescsi_pc_t *pc = (idescsi_pc_t *) rq->buffer;
+	idescsi_pc_t *pc = (idescsi_pc_t *) rq->special;
 	int log = test_bit(IDESCSI_LOG_CMD, &scsi->log);
 	struct Scsi_Host *host;
 	u8 *scsi_buf;
@@ -464,7 +464,7 @@
 #endif /* IDESCSI_DEBUG_LOG */
 
 	if (rq->flags & REQ_SPECIAL) {
-		return idescsi_issue_pc (drive, (idescsi_pc_t *) rq->buffer);
+		return idescsi_issue_pc (drive, (idescsi_pc_t *) rq->special);
 	}
 	blk_dump_rq_flags(rq, "ide-scsi: unsup command");
 	idescsi_end_request (0,HWGROUP (drive));
@@ -662,6 +662,7 @@
 	if ((first_bh = bhp = bh = bio_alloc(GFP_ATOMIC, 1)) == NULL)
 		goto abort;
 	bio_init(bh);
+	bh->bi_vcnt = 1;
 	while (--count) {
 		if ((bh = bio_alloc(GFP_ATOMIC, 1)) == NULL)
 			goto abort;
@@ -802,7 +803,7 @@
 	}
 
 	ide_init_drive_cmd (rq);
-	rq->buffer = (char *) pc;
+	rq->special = (char *) pc;
 	rq->bio = idescsi_dma_bio (drive, pc);
 	rq->flags = REQ_SPECIAL;
 	spin_unlock(&cmd->host->host_lock);
diff -ur -X exclude /opt/kernel/linux-2.5.1-pre9/drivers/scsi/scsi_merge.c linux/drivers/scsi/scsi_merge.c
--- /opt/kernel/linux-2.5.1-pre9/drivers/scsi/scsi_merge.c	Tue Dec 11 05:01:51 2001
+++ linux/drivers/scsi/scsi_merge.c	Tue Dec 11 11:44:03 2001
@@ -205,8 +205,10 @@
 
 static inline int scsi_new_mergeable(request_queue_t * q,
 				     struct request * req,
-				     int nr_segs)
+				     struct bio *bio)
 {
+	int nr_segs = bio_hw_segments(q, bio);
+
 	/*
 	 * pci_map_sg will be able to merge these two
 	 * into a single hardware sg entry, check if
@@ -223,8 +225,9 @@
 
 static inline int scsi_new_segment(request_queue_t * q,
 				   struct request * req,
-				   struct bio *bio, int nr_segs)
+				   struct bio *bio)
 {
+	int nr_segs = bio_hw_segments(q, bio);
 	/*
 	 * pci_map_sg won't be able to map these two
 	 * into a single hardware sg entry, so we have to
@@ -244,8 +247,10 @@
 
 static inline int scsi_new_segment(request_queue_t * q,
 				   struct request * req,
-				   struct bio *bio, int nr_segs)
+				   struct bio *bio)
 {
+	int nr_segs = bio_hw_segments(q, bio);
+
 	if (req->nr_segments + nr_segs > q->max_segments) {
 		req->flags |= REQ_NOMERGE;
 		return 0;
@@ -296,45 +301,33 @@
 					 struct request *req,
 					 struct bio *bio)
 {
-	int bio_segs;
-
 	if (req->nr_sectors + bio_sectors(bio) > q->max_sectors) {
 		req->flags |= REQ_NOMERGE;
 		return 0;
 	}
 
-	bio_segs = bio_hw_segments(q, bio);
-	if (blk_contig_segment(q, req->biotail, bio))
-		bio_segs--;
-
 #ifdef DMA_CHUNK_SIZE
 	if (MERGEABLE_BUFFERS(bio, req->bio))
-		return scsi_new_mergeable(q, req, bio_segs);
+		return scsi_new_mergeable(q, req, bio);
 #endif
 
-	return scsi_new_segment(q, req, bio, bio_segs);
+	return scsi_new_segment(q, req, bio);
 }
 
 __inline static int __scsi_front_merge_fn(request_queue_t * q,
 					  struct request *req,
 					  struct bio *bio)
 {
-	int bio_segs;
-
 	if (req->nr_sectors + bio_sectors(bio) > q->max_sectors) {
 		req->flags |= REQ_NOMERGE;
 		return 0;
 	}
 
-	bio_segs = bio_hw_segments(q, bio);
-	if (blk_contig_segment(q, req->biotail, bio))
-		bio_segs--;
-
 #ifdef DMA_CHUNK_SIZE
 	if (MERGEABLE_BUFFERS(bio, req->bio))
-		return scsi_new_mergeable(q, req, bio_segs);
+		return scsi_new_mergeable(q, req, bio);
 #endif
-	return scsi_new_segment(q, req, bio, bio_segs);
+	return scsi_new_segment(q, req, bio);
 }
 
 /*
@@ -370,32 +363,23 @@
 MERGEFCT(scsi_front_merge_fn, front)
 
 /*
- * Function:    __scsi_merge_requests_fn()
+ * Function:    scsi_merge_requests_fn_()
  *
- * Purpose:     Prototype for queue merge function.
+ * Purpose:     queue merge function.
  *
  * Arguments:   q       - Queue for which we are merging request.
  *              req     - request into which we wish to merge.
- *              next    - 2nd request that we might want to combine with req
- *              dma_host - 1 if this host has ISA DMA issues (bus doesn't
- *                      expose all of the address lines, so that DMA cannot
- *                      be done from an arbitrary address).
+ *              next    - Block which we may wish to merge into request
  *
- * Returns:     1 if it is OK to merge the two requests.  0
+ * Returns:     1 if it is OK to merge the block into the request.  0
  *              if it is not OK.
  *
  * Lock status: queue lock is assumed to be held here.
  *
- * Notes:       Some drivers have limited scatter-gather table sizes, and
- *              thus they cannot queue an infinitely large command.  This
- *              function is called from ll_rw_blk before it attempts to merge
- *              a new block into a request to make sure that the request will
- *              not become too large.
  */
-__inline static int __scsi_merge_requests_fn(request_queue_t * q,
-					     struct request *req,
-					     struct request *next,
-					     int dma_host)
+inline static int scsi_merge_requests_fn(request_queue_t * q,
+					 struct request *req,
+					 struct request *next)
 {
 	int bio_segs;
 
@@ -446,35 +430,6 @@
 }
 
 /*
- * Function:    scsi_merge_requests_fn_()
- *
- * Purpose:     queue merge function.
- *
- * Arguments:   q       - Queue for which we are merging request.
- *              req     - request into which we wish to merge.
- *              bio     - Block which we may wish to merge into request
- *
- * Returns:     1 if it is OK to merge the block into the request.  0
- *              if it is not OK.
- *
- * Lock status: queue lock is assumed to be held here.
- *
- * Notes:       Optimized for different cases depending upon whether
- *              ISA DMA is in use and whether clustering should be used.
- */
-#define MERGEREQFCT(_FUNCTION, _DMA)			\
-static int _FUNCTION(request_queue_t * q,		\
-		     struct request * req,		\
-		     struct request * next)		\
-{							\
-    int ret;						\
-    ret =  __scsi_merge_requests_fn(q, req, next, _DMA); \
-    return ret;						\
-}
-
-MERGEREQFCT(scsi_merge_requests_fn_, 0)
-MERGEREQFCT(scsi_merge_requests_fn_d, 1)
-/*
  * Function:    __init_io()
  *
  * Purpose:     Prototype for io initialize function.
@@ -811,15 +766,13 @@
 	 * is simply easier to do it ourselves with our own functions
 	 * rather than rely upon the default behavior of ll_rw_blk.
 	 */
+	q->back_merge_fn = scsi_back_merge_fn;
+	q->front_merge_fn = scsi_front_merge_fn;
+	q->merge_requests_fn = scsi_merge_requests_fn;
+
 	if (SHpnt->unchecked_isa_dma == 0) {
-		q->back_merge_fn = scsi_back_merge_fn;
-		q->front_merge_fn = scsi_front_merge_fn;
-		q->merge_requests_fn = scsi_merge_requests_fn_;
 		SDpnt->scsi_init_io_fn = scsi_init_io_v;
 	} else {
-		q->back_merge_fn = scsi_back_merge_fn;
-		q->front_merge_fn = scsi_front_merge_fn;
-		q->merge_requests_fn = scsi_merge_requests_fn_d;
 		SDpnt->scsi_init_io_fn = scsi_init_io_vd;
 	}
 
diff -ur -X exclude /opt/kernel/linux-2.5.1-pre9/fs/bio.c linux/fs/bio.c
--- /opt/kernel/linux-2.5.1-pre9/fs/bio.c	Tue Dec 11 05:01:51 2001
+++ linux/fs/bio.c	Tue Dec 11 05:31:25 2001
@@ -481,32 +481,6 @@
 	return 0;
 }
 
-/*
- * obviously doesn't work for stacking drivers, but ll_rw_blk will split
- * bio for those
- */
-int get_max_segments(kdev_t dev)
-{
-	int segments = MAX_SEGMENTS;
-	request_queue_t *q;
-
-	if ((q = blk_get_queue(dev)))
-		segments = q->max_segments;
-
-	return segments;
-}
-
-int get_max_sectors(kdev_t dev)
-{
-	int sectors = MAX_SECTORS;
-	request_queue_t *q;
-
-	if ((q = blk_get_queue(dev)))
-		sectors = q->max_sectors;
-
-	return sectors;
-}
-
 /**
  * ll_rw_kio - submit a &struct kiobuf for I/O
  * @rw:   %READ or %WRITE
@@ -522,7 +496,6 @@
 void ll_rw_kio(int rw, struct kiobuf *kio, kdev_t dev, sector_t sector)
 {
 	int i, offset, size, err, map_i, total_nr_pages, nr_pages;
-	int max_bytes, max_segments;
 	struct bio_vec *bvec;
 	struct bio *bio;
 
@@ -539,19 +512,6 @@
 	}
 
 	/*
-	 * rudimentary max sectors/segments checks and setup. once we are
-	 * sure that drivers can handle requests that cannot be completed in
-	 * one go this will die
-	 */
-	max_bytes = get_max_sectors(dev) << 9;
-	max_segments = get_max_segments(dev);
-	if ((max_bytes >> PAGE_SHIFT) < (max_segments + 1))
-		max_segments = (max_bytes >> PAGE_SHIFT);
-
-	if (max_segments > BIO_MAX_PAGES)
-		max_segments = BIO_MAX_PAGES;
-
-	/*
 	 * maybe kio is bigger than the max we can easily map into a bio.
 	 * if so, split it up in appropriately sized chunks.
 	 */
@@ -564,9 +524,11 @@
 	map_i = 0;
 
 next_chunk:
+	nr_pages = BIO_MAX_SECTORS >> (PAGE_SHIFT - 9);
+	if (nr_pages > total_nr_pages)
+		nr_pages = total_nr_pages;
+
 	atomic_inc(&kio->io_count);
-	if ((nr_pages = total_nr_pages) > max_segments)
-		nr_pages = max_segments;
 
 	/*
 	 * allocate bio and do initial setup
@@ -591,7 +553,7 @@
 
 		BUG_ON(kio->maplist[map_i] == NULL);
 
-		if (bio->bi_size + nbytes > max_bytes)
+		if (bio->bi_size + nbytes > (BIO_MAX_SECTORS << 9))
 			goto queue_io;
 
 		bio->bi_vcnt++;
@@ -714,3 +676,4 @@
 EXPORT_SYMBOL(bio_copy);
 EXPORT_SYMBOL(__bio_clone);
 EXPORT_SYMBOL(bio_clone);
+EXPORT_SYMBOL(bio_hw_segments);
diff -ur -X exclude /opt/kernel/linux-2.5.1-pre9/fs/partitions/acorn.c linux/fs/partitions/acorn.c
--- /opt/kernel/linux-2.5.1-pre9/fs/partitions/acorn.c	Mon Oct  1 23:03:26 2001
+++ linux/fs/partitions/acorn.c	Tue Dec 11 06:39:47 2001
@@ -162,12 +162,12 @@
 		struct adfs_discrecord *dr;
 		unsigned int nr_sects;
 
-		if (!(minor & mask))
-			break;
-
 		data = read_dev_sector(bdev, start_blk * 2 + 6, &sect);
 		if (!data)
 			return -1;
+
+		if (!(minor & mask))
+			break;
 
 		dr = adfs_partition(hd, name, data, first_sector, minor++);
 		if (!dr)
diff -ur -X exclude /opt/kernel/linux-2.5.1-pre9/fs/partitions/check.h linux/fs/partitions/check.h
--- /opt/kernel/linux-2.5.1-pre9/fs/partitions/check.h	Tue Dec 11 05:01:51 2001
+++ linux/fs/partitions/check.h	Tue Dec 11 06:43:31 2001
@@ -1,3 +1,5 @@
+#include <linux/pagemap.h>
+
 /*
  * add_gd_partition adds a partitions details to the devices partition
  * description.
diff -ur -X exclude /opt/kernel/linux-2.5.1-pre9/include/linux/bio.h linux/include/linux/bio.h
--- /opt/kernel/linux-2.5.1-pre9/include/linux/bio.h	Tue Dec 11 05:01:51 2001
+++ linux/include/linux/bio.h	Tue Dec 11 05:50:25 2001
@@ -28,6 +28,8 @@
 #define BIO_BUG_ON
 #endif
 
+#define BIO_MAX_SECTORS	128
+
 /*
  * was unsigned short, but we might as well be ready for > 64kB I/O pages
  */
@@ -60,7 +62,7 @@
 	unsigned short		bi_vcnt;	/* how many bio_vec's */
 	unsigned short		bi_idx;		/* current index into bvl_vec */
 	unsigned short		bi_hw_seg;	/* actual mapped segments */
-	unsigned int		bi_size;	/* total size in bytes */
+	unsigned int		bi_size;	/* residual I/O count */
 	unsigned int		bi_max;		/* max bvl_vecs we can hold,
 						   used as index into pool */
 

-- 
Jens Axboe