Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ata: libata-sata: Improve ata_change_queue_depth()

ata_change_queue_depth() implements different behaviors for ATA devices
managed by libsas than for those managed by libata directly.
Specifically, if a user attempts to set a device queue depth to a value
larger than 32 (ATA_MAX_QUEUE), the queue depth is capped to the maximum
and set to 32 for libsas managed devices whereas for libata managed
devices, the queue depth is unchanged and an error returned to the user.
This is due to the fact that for libsas devices, sdev->host->can_queue
may indicate the host (HBA) maximum number of commands that can be
queued rather than the device maximum queue depth.

Change ata_change_queue_depth() to provide a consistent behavior for all
devices by changing the queue depth capping code to a check that the
user provided value does not exceed the device maximum queue depth.
This check is moved before the code clearing or setting the
ATA_DFLAG_NCQ_OFF flag to ensure that this flag is not modified when an
invlaid queue depth is provided.

While at it, two other small improvements are added:
1) Use ata_ncq_supported() instead of ata_ncq_enabled() and clear the
ATA_DFLAG_NCQ_OFF flag only and only if needed.
2) If the user provided queue depth is equal to the current queue depth,
do not return an error as that is useless.

Overall, the behavior of ata_change_queue_depth() for libata managed
devices is unchanged. The behavior with libsas managed devices becomes
consistent with libata managed devices.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

+21 -10
+21 -10
drivers/ata/libata-sata.c
··· 1035 1035 { 1036 1036 struct ata_device *dev; 1037 1037 unsigned long flags; 1038 + int max_queue_depth; 1038 1039 1039 1040 spin_lock_irqsave(ap->lock, flags); 1040 1041 ··· 1045 1044 return sdev->queue_depth; 1046 1045 } 1047 1046 1048 - /* NCQ enabled? */ 1049 - dev->flags &= ~ATA_DFLAG_NCQ_OFF; 1050 - if (queue_depth == 1 || !ata_ncq_enabled(dev)) { 1047 + /* 1048 + * Make sure that the queue depth requested does not exceed the device 1049 + * capabilities. 1050 + */ 1051 + max_queue_depth = min(ATA_MAX_QUEUE, sdev->host->can_queue); 1052 + max_queue_depth = min(max_queue_depth, ata_id_queue_depth(dev->id)); 1053 + if (queue_depth > max_queue_depth) { 1054 + spin_unlock_irqrestore(ap->lock, flags); 1055 + return -EINVAL; 1056 + } 1057 + 1058 + /* 1059 + * If NCQ is not supported by the device or if the target queue depth 1060 + * is 1 (to disable drive side command queueing), turn off NCQ. 1061 + */ 1062 + if (queue_depth == 1 || !ata_ncq_supported(dev)) { 1051 1063 dev->flags |= ATA_DFLAG_NCQ_OFF; 1052 1064 queue_depth = 1; 1065 + } else { 1066 + dev->flags &= ~ATA_DFLAG_NCQ_OFF; 1053 1067 } 1054 1068 1055 1069 spin_unlock_irqrestore(ap->lock, flags); 1056 1070 1057 - /* limit and apply queue depth */ 1058 - queue_depth = min(queue_depth, sdev->host->can_queue); 1059 - queue_depth = min(queue_depth, ata_id_queue_depth(dev->id)); 1060 - queue_depth = min(queue_depth, ATA_MAX_QUEUE); 1061 - 1062 - if (sdev->queue_depth == queue_depth) 1063 - return -EINVAL; 1071 + if (queue_depth == sdev->queue_depth) 1072 + return sdev->queue_depth; 1064 1073 1065 1074 return scsi_change_queue_depth(sdev, queue_depth); 1066 1075 }