Re: [PATCH] ibmvfc: Add max_sectors module parameter

[Brian King <brking@linux.vnet.ibm.com>]

On 8/12/24 3:22 PM, Martin Wilck wrote:
> On Tue, 2024-07-30 at 12:51 -0500, Brian King wrote:
>> There are some scenarios that can occur, such as performing an
>> upgrade of the virtual I/O server, where the supported max transfer
>> of the backing device for an ibmvfc HBA can change. If the max
>> transfer of the backing device decreases, this can cause issues with
>> previously discovered LUNs. This patch accomplishes two things.
>> First, it changes the default ibmvfc max transfer value to 1MB.
>> This is generally supported by all backing devices, which should
>> mitigate this issue out of the box. Secondly, it adds a module
>> parameter, enabling a user to increase the max transfer value to
>> values that are larger than 1MB, as long as they have configured
>> these larger values on the virtual I/O server as well.
>>
>> Signed-off-by: Brian King <brking@linux.ibm.com>
>> ---
>>  drivers/scsi/ibmvscsi/ibmvfc.c | 10 +++++++---
>>  drivers/scsi/ibmvscsi/ibmvfc.h |  2 +-
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c
>> b/drivers/scsi/ibmvscsi/ibmvfc.c
>> index a3d1013c8307..611901562e06 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>> @@ -37,6 +37,7 @@ static unsigned int default_timeout =
>> IBMVFC_DEFAULT_TIMEOUT;
>>  static u64 max_lun = IBMVFC_MAX_LUN;
>>  static unsigned int max_targets = IBMVFC_MAX_TARGETS;
>>  static unsigned int max_requests = IBMVFC_MAX_REQUESTS_DEFAULT;
>> +static unsigned int max_sectors = IBMVFC_MAX_SECTORS;
>>  static u16 scsi_qdepth = IBMVFC_SCSI_QDEPTH;
>>  static unsigned int disc_threads = IBMVFC_MAX_DISC_THREADS;
>>  static unsigned int ibmvfc_debug = IBMVFC_DEBUG;
>> @@ -83,6 +84,9 @@ MODULE_PARM_DESC(default_timeout,
>>  module_param_named(max_requests, max_requests, uint, S_IRUGO);
>>  MODULE_PARM_DESC(max_requests, "Maximum requests for this adapter. "
>>  		 "[Default="
>> __stringify(IBMVFC_MAX_REQUESTS_DEFAULT) "]");
>> +module_param_named(max_sectors, max_sectors, uint, S_IRUGO);
>> +MODULE_PARM_DESC(max_sectors, "Maximum sectors for this adapter. "
>> +		 "[Default=" __stringify(IBMVFC_MAX_SECTORS) "]");
>>  module_param_named(scsi_qdepth, scsi_qdepth, ushort, S_IRUGO);
>>  MODULE_PARM_DESC(scsi_qdepth, "Maximum scsi command depth per
>> adapter queue. "
>>  		 "[Default=" __stringify(IBMVFC_SCSI_QDEPTH) "]");
>> @@ -1494,7 +1498,7 @@ static void ibmvfc_set_login_info(struct
>> ibmvfc_host *vhost)
>>  	memset(login_info, 0, sizeof(*login_info));
>>  
>>  	login_info->ostype = cpu_to_be32(IBMVFC_OS_LINUX);
>> -	login_info->max_dma_len = cpu_to_be64(IBMVFC_MAX_SECTORS <<
>> 9);
>> +	login_info->max_dma_len = cpu_to_be64(max_sectors << 9);
>>  	login_info->max_payload = cpu_to_be32(sizeof(struct
>> ibmvfc_fcp_cmd_iu));
>>  	login_info->max_response = cpu_to_be32(sizeof(struct
>> ibmvfc_fcp_rsp));
>>  	login_info->partition_num = cpu_to_be32(vhost-
>>> partition_number);
>> @@ -5230,7 +5234,7 @@ static void ibmvfc_npiv_login_done(struct
>> ibmvfc_event *evt)
>>  	}
>>  
>>  	vhost->logged_in = 1;
>> -	npiv_max_sectors = min((uint)(be64_to_cpu(rsp->max_dma_len)
>>>> 9), IBMVFC_MAX_SECTORS);
>> +	npiv_max_sectors = min((uint)(be64_to_cpu(rsp->max_dma_len)
>>>> 9), max_sectors);
>>  	dev_info(vhost->dev, "Host partition: %s, device: %s %s %s
>> max sectors %u\n",
>>  		 rsp->partition_name, rsp->device_name, rsp-
>>> port_loc_code,
>>  		 rsp->drc_name, npiv_max_sectors);
>> @@ -6329,7 +6333,7 @@ static int ibmvfc_probe(struct vio_dev *vdev,
>> const struct vio_device_id *id)
>>  	shost->can_queue = scsi_qdepth;
>>  	shost->max_lun = max_lun;
>>  	shost->max_id = max_targets;
>> -	shost->max_sectors = IBMVFC_MAX_SECTORS;
>> +	shost->max_sectors = max_sectors;
> 
> Would it make sense to check whether the user-provided max_sectors
> value is within some reasonable limits?

Agreed.  I'll follow up with an updated version.

Thanks,

Brian


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center