Re: Scsi problems in 2.5.1-pre9

Jens Axboe (axboe@suse.de)
Tue, 11 Dec 2001 17:54:26 +0100


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

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/