On 1/18/21 12:34 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.
>
> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
> ---
> drivers/scsi/libiscsi.c | 84 +++++++++++++++++++++++++----------------
> include/scsi/libiscsi.h | 2 +
> 2 files changed, 54 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index b271d3accd2a..195006a08e0d 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -2648,6 +2648,54 @@ 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;
> +
> +check:
> + 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 max cmds of %d. 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 max cmds of %d. 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 max cmds of %d. Must be a power of 2.\n",
> + total_cmds);
I don't like mixing KERN_ERR and KERN_INFO for the same event. Can we
make it just one KERN_INFO? (since we keep going)
> + total_cmds = rounddown_pow_of_two(total_cmds);
> + if (total_cmds < ISCSI_TOTAL_CMDS_MIN)
> + return -EINVAL;
> + printk(KERN_INFO "iscsi: Rounding max cmds to %d.\n",
> + total_cmds);
> + }
> +
> + scsi_cmds = total_cmds - ISCSI_MGMT_CMDS_MAX;
> + if (shost->can_queue && scsi_cmds > shost->can_queue) {
> + total_cmds = shost->can_queue;
> +
> + printk(KERN_INFO "iscsi: requested max cmds %u is higher than driver limit. Using driver limit %u\n",
> + requested_cmds_max, shost->can_queue);
> + goto check;
> + }
> +
> + return scsi_cmds;
> +}
> +EXPORT_SYMBOL_GPL(iscsi_host_get_max_scsi_cmds);
> +
> /**
> * iscsi_host_add - add host to system
> * @shost: scsi host
> @@ -2801,7 +2849,7 @@ iscsi_session_setup(struct iscsi_transport *iscsit, struct Scsi_Host *shost,
> struct iscsi_host *ihost = shost_priv(shost);
> struct iscsi_session *session;
> struct iscsi_cls_session *cls_session;
> - int cmd_i, scsi_cmds, total_cmds = cmds_max;
> + int cmd_i, scsi_cmds;
> unsigned long flags;
>
> spin_lock_irqsave(&ihost->lock, flags);
> @@ -2812,37 +2860,9 @@ iscsi_session_setup(struct iscsi_transport *iscsit, struct Scsi_Host *shost,
> ihost->num_sessions++;
> spin_unlock_irqrestore(&ihost->lock, flags);
>
> - 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);
> + scsi_cmds = iscsi_host_get_max_scsi_cmds(shost, cmds_max);
> + if (scsi_cmds < 0)
> goto dec_session_count;
> - }
> -
> - 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",
> - 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)
> - goto dec_session_count;
> - printk(KERN_INFO "iscsi: Rounding can_queue to %d.\n",
> - total_cmds);
> - }
> - scsi_cmds = total_cmds - ISCSI_MGMT_CMDS_MAX;
>
> cls_session = iscsi_alloc_session(shost, iscsit,
> sizeof(struct iscsi_session) +
> @@ -2858,7 +2878,7 @@ iscsi_session_setup(struct iscsi_transport *iscsit, struct Scsi_Host *shost,
> session->lu_reset_timeout = 15;
> session->abort_timeout = 10;
> session->scsi_cmds_max = scsi_cmds;
> - session->cmds_max = total_cmds;
> + session->cmds_max = scsi_cmds + ISCSI_MGMT_CMDS_MAX;
> session->queued_cmdsn = session->cmdsn = initial_cmdsn;
> session->exp_cmdsn = initial_cmdsn + 1;
> session->max_cmdsn = initial_cmdsn + 1;
> diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
> index 44a9554aea62..02f966e9358f 100644
> --- a/include/scsi/libiscsi.h
> +++ b/include/scsi/libiscsi.h
> @@ -395,6 +395,8 @@ extern struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht,
> extern void iscsi_host_remove(struct Scsi_Host *shost);
> extern void iscsi_host_free(struct Scsi_Host *shost);
> extern int iscsi_target_alloc(struct scsi_target *starget);
> +extern int iscsi_host_get_max_scsi_cmds(struct Scsi_Host *shost,
> + uint16_t requested_cmds_max);
>
> /*
> * session management
>
If you choose to change the KERN_ERR I mentioned above, please add my
reviwed-by tag.
