Re: [PATCH v2] add bidi support for block pc requests

[Boaz Harrosh <bharrosh@panasas.com>]

James Bottomley wrote:
> On Thu, 2007-05-17 at 11:49 +0300, Boaz Harrosh wrote:
>> These are regular fs (ext3) requests during bootup. The machine will not
>> boot. (Usually from the read ahead code)
>> Don't believe me look at the second patch Over Tomo's cleanup.
>> If I define SCSI_MAX_SG_SEGMENTS to 127 it will crash even when I
>> did in code:
>> 	blk_queue_max_phys_segments(q, SCSI_MAX_SG_SEGMENTS);
>> I suppose someone is looking at a different definition. Or there is
>> another call I need to do for this to work.
> 
> It would really help us if you showed the actual code for what you did
> and where ... if this is wrong, we have bigger problems that quibbling
> about bidirectional slab sizes.  The correct way to adjust this limit
> artificially to 127 is:
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 1f5a07b..4a27841 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1576,7 +1576,7 @@ struct request_queue *__scsi_alloc_queue(struct Scsi_Host *shost,
>  		return NULL;
>  
>  	blk_queue_max_hw_segments(q, shost->sg_tablesize);
> -	blk_queue_max_phys_segments(q, SCSI_MAX_PHYS_SEGMENTS);
> +	blk_queue_max_phys_segments(q, 127);
>  	blk_queue_max_sectors(q, shost->max_sectors);
>  	blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost));
>  	blk_queue_segment_boundary(q, shost->dma_boundary);
> 
> (It doesn't alter the allocation pools or anything else, just limits the
> max phys segments of the queue).  The way to check that this limit is
> being honoured is:
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 1f5a07b..ae42e4d 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1000,6 +1000,7 @@ static int scsi_init_io(struct scsi_cmnd *cmd)
>  	 * kmapping pages)
>  	 */
>  	cmd->use_sg = req->nr_phys_segments;
> +	WARN_ON(req->nr_phys_segments > 127);
>  
>  	/*
>  	 * If sg table allocation fails, requeue request later.
> 
> James
> 
> 
Yes Tomo found it at ata_scsi_slave_config(). Attached below the way I
fixed it. Now it works with 127.

(By the way Tomo, a printk like you did in scsi_init_io and
scsi_free_sgtable, 2 for every IO, or even 1 for every IO, will make
a SCSI booting PC like mine almost un-usable. Think of printk going
to log-file doing a printk...)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index dd81fa7..de8c796 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -800,7 +800,9 @@ int ata_scsi_slave_config(struct scsi_device *sdev)

 	ata_scsi_sdev_config(sdev);

-	blk_queue_max_phys_segments(sdev->request_queue, LIBATA_MAX_PRD);
+	if ( sdev->request_queue->max_phys_segments > LIBATA_MAX_PRD )
+		blk_queue_max_phys_segments(sdev->request_queue,
+		                                       LIBATA_MAX_PRD);

 	sdev->manage_start_stop = 1;