On 1/5/21 5:18 PM, Mike Christie wrote:
> 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,
Correction on that. With he current code we can hit zero for iscsi_tcp
and old tools with iser. I mentioned in the other email I'll fix that
up.
