On 1/5/21 4:53 PM, Lee Duncan wrote:
> On 12/20/20 6:37 PM, Mike Christie wrote:
>> This patch just breaks out the code that calculates the number
>> of scsi cmds that will be used for a scsi session. It also adds
>> a check that we don't go over the host's can_queue value.
>
> I'm curious. It's a "good thing" to check the command count in a better
> way now, but was there any known instance of the count miscalculation in
> the current code causing issues?
No one has hit any issues. It's so userspace knows it's not going to
get the requested value.
>
>>
>> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
>> ---
>> drivers/scsi/libiscsi.c | 81 ++++++++++++++++++++++++++++++-------------------
>> include/scsi/libiscsi.h | 2 ++
>> 2 files changed, 51 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
>> index 796465e..f1ade91 100644
>> --- a/drivers/scsi/libiscsi.c
>> +++ b/drivers/scsi/libiscsi.c
>> @@ -2648,6 +2648,51 @@ void iscsi_pool_free(struct iscsi_pool *q)
>> }
>> EXPORT_SYMBOL_GPL(iscsi_pool_free);
>>
>> +int iscsi_host_get_max_scsi_cmds(struct Scsi_Host *shost,
>> + uint16_t requested_cmds_max)
>> +{
>> + int scsi_cmds, total_cmds = requested_cmds_max;
>> +
>> + if (!total_cmds)
>> + total_cmds = ISCSI_DEF_XMIT_CMDS_MAX;
>> + /*
>> + * The iscsi layer needs some tasks for nop handling and tmfs,
>> + * so the cmds_max must at least be greater than ISCSI_MGMT_CMDS_MAX
>> + * + 1 command for scsi IO.
>> + */
>> + if (total_cmds < ISCSI_TOTAL_CMDS_MIN) {
>> + printk(KERN_ERR "iscsi: invalid can_queue of %d. can_queue must be a power of two that is at least %d.\n",
>> + total_cmds, ISCSI_TOTAL_CMDS_MIN);
>> + return -EINVAL;
>> + }
>> +
>> + if (total_cmds > ISCSI_TOTAL_CMDS_MAX) {
>> + printk(KERN_ERR "iscsi: invalid can_queue of %d. can_queue must be a power of 2 less than or equal to %d.\n",
>> + requested_cmds_max, ISCSI_TOTAL_CMDS_MAX);
>> + total_cmds = ISCSI_TOTAL_CMDS_MAX;
>> + }
>> +
>> + if (!is_power_of_2(total_cmds)) {
>> + printk(KERN_ERR "iscsi: invalid can_queue of %d. can_queue must be a power of 2.\n",
>> + total_cmds);
>> + total_cmds = rounddown_pow_of_two(total_cmds);
>> + if (total_cmds < ISCSI_TOTAL_CMDS_MIN)
>> + return -EINVAL;
>> + printk(KERN_INFO "iscsi: Rounding can_queue to %d.\n",
>> + total_cmds);
>> + }
>> +
>> + scsi_cmds = total_cmds - ISCSI_MGMT_CMDS_MAX;
>> + if (shost->can_queue && scsi_cmds > shost->can_queue) {
>> + scsi_cmds = shost->can_queue - ISCSI_MGMT_CMDS_MAX;
>> + printk(KERN_INFO "iscsi: requested cmds_max %u higher than driver limit. Using driver max %u\n",
>> + requested_cmds_max, shost->can_queue);
>> + }
>
> If the device can queue, what if "can_queue" is equal to or less than
> ISCSI_MGMT_CMDS_MAX?
>
It wouldn't be possible, because the drivers set their can_queue a lot higher
than ISCSI_MGMT_CMDS_MAX, but for this and the other comment I'll fix up the
check/code.
