On Fri, Jan 27, 2017 at 11:28:42AM -0800, Raghava Aditya Renukunta wrote:
> Added support to set qd of drives in slave_configure.This only works for
> HBA1000 attached drives.
> 
> Signed-off-by: Raghava Aditya Renukunta 
> <[email protected]>
> Signed-off-by: Dave Carroll <[email protected]>
> 
> ---

[...]

> @@ -428,34 +441,41 @@ static int aac_slave_configure(struct scsi_device *sdev)
>                */
>               if (sdev->request_queue->rq_timeout < (45 * HZ))
>                       blk_queue_rq_timeout(sdev->request_queue, 45*HZ);
> -             for (cid = 0; cid < aac->maximum_num_containers; ++cid)
> -                     if (aac->fsa_dev[cid].valid)
> -                             ++num_lsu;
> -             __shost_for_each_device(dev, host) {
> -                     if (dev->tagged_supported && (dev->type == TYPE_DISK) &&
> +             if (!is_native_device) {
> +                     for (cid = 0; cid < aac->maximum_num_containers; ++cid)
> +                             if (aac->fsa_dev[cid].valid)
> +                                     ++num_lsu;
> +                     __shost_for_each_device(dev, host) {
> +                             if (dev->tagged_supported &&
> +                                     (dev->type == TYPE_DISK) &&
>                                       (!aac->raid_scsi_mode ||
> -                                             (sdev_channel(sdev) != 2)) &&
> +                                     (sdev_channel(sdev) != 2)) &&
>                                       !dev->no_uld_attach) {
> -                             if ((sdev_channel(dev) != CONTAINER_CHANNEL)
> -                              || !aac->fsa_dev[sdev_id(dev)].valid)
> -                                     ++num_lsu;
> -                     } else
> -                             ++num_one;
> +                                     if ((sdev_channel(dev)
> +                                             != CONTAINER_CHANNEL)
> +                                      || !aac->fsa_dev[sdev_id(dev)].valid) {
> +                                             ++num_lsu;
> +                                     }
> +                             } else {
> +                                     ++num_one;
> +                             }
> +                     }
> +                     if (num_lsu == 0)
> +                             ++num_lsu;
> +                     depth = (host->can_queue - num_one) / num_lsu;
> +                     if (depth > 256)
> +                             depth = 256;
> +                     else if (depth < 2)
> +                             depth = 2;
> +                     scsi_change_queue_depth(sdev, depth);
> +             } else {
> +                     scsi_change_queue_depth(sdev,
> +                             aac->hba_map[chn][tid].qd_limit);

At least in diff form, the above is quite hard to read. Can you make the code
flow a bit more obvious? The extra level of indentation and thus newly
introduced linebreaks didn't make it easier.


>               }
> -             if (num_lsu == 0)
> -                     ++num_lsu;
> -             depth = (host->can_queue - num_one) / num_lsu;
> -             if (depth > 256)
> -                     depth = 256;
> -             else if (depth < 2)
> -                     depth = 2;
> -             scsi_change_queue_depth(sdev, depth);
> -     } else {
> -             scsi_change_queue_depth(sdev, 1);
> -
> -             sdev->tagged_supported = 1;
>       }


[...]

Thanks,
        Johannes
-- 
Johannes Thumshirn                                          Storage
[email protected]                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to