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

[Boaz Harrosh <bharrosh@panasas.com>]

Jeff Garzik wrote:
> Boaz Harrosh wrote:
>> FUJITA Tomonori wrote:
>>> From: Boaz Harrosh <bharrosh@panasas.com>
>>> Subject: Re: [PATCH v2] add bidi support for block pc requests
>>> Date: Thu, 17 May 2007 17:00:21 +0300
>>>
>>>> Yes Tomo found it at ata_scsi_slave_config(). Attached below the way I
>>>> fixed it. Now it works with 127.
>>> I think that we can just remove blk_queue_max_phys_segments since the
>>> ata drivers seem to set sg_tablesize to LIBATA_MAX_PRD.
>>>
>> Who should send a patch upstream? (I cc'ed Jeff Garzik)
>>
>> Boaz
>>
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index dd81fa7..3660f3e 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -800,8 +800,6 @@ 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);
>> -
>>  	sdev->manage_start_stop = 1;
> 
> I don't mind the patch, but could someone refresh me as to the context?
> 
> Is there something wrong with the above code, or is it simply redundant 
> to the scsi_host_template settings in each LLDD?
> 
> 	Jeff
> 
> 
> 

Hi Jeff
What happens is that if SCSI-ml sets an higher value than LIBATA_MAX_PRD (=128)
than every thing is OK and libata-scsi will only see its LIBATA_MAX_PRD. But what
happens if SCSI-ml sets a lower value? It will than crash on unexpected high sg
count. My first Patch was an "if >" but Tomo said that it is redundant since
drivers do that already. So I guess it is your call. Can it be removed or we need
a: if ( sdev->request_queue->max_phys_segments > LIBATA_MAX_PRD)

	Boaz