On 14-06-11 02:53 AM, Bart Van Assche wrote:
On 06/11/14 08:34, Hannes Reinecke wrote:
On 06/10/2014 07:58 PM, Bart Van Assche wrote:
Many SCSI LLD's use int_to_scsilun() in the hot path (queuecommand()).
This patch series makes the int_to_scsilun() function slightly more
expensive. Has it been considered to cache the result of int_to_scsilun()
such that LLD's can copy the cached int_to_scsilun() result instead of
having to call int_to_scsilun() in the queuecommand() function ?
Something like the (untested) patch below might be sufficient:
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e02b3aa..9e50d78 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -244,6 +244,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
scsi_target *starget,
sdev->queue_ramp_up_period = SCSI_DEFAULT_RAMP_UP_PERIOD;
sdev->id = starget->id;
sdev->lun = lun;
+ int_to_scsilun(lun, &sdev->scsi_lun);
sdev->channel = starget->channel;
sdev->sdev_state = SDEV_CREATED;
INIT_LIST_HEAD(&sdev->siblings);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5853c91..48ea68e 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -100,6 +100,8 @@ struct scsi_device {
unsigned int id, lun, channel;
+ struct scsi_lun scsi_lun; /* int_to_scsilun(lun) */
+
unsigned int manufacturer; /* Manufacturer of device, for using
* vendor-specific cmd's */
unsigned sector_size; /* size in bytes */
Thanks,
Hmm. No, so far it hasn't been considered. Maybe it's even worthwhile to
move the situation around, ie using primarily the 64-bit LUN and only
convert it into an integer if so requested.
But yeah, it's definitely something we should look into.
Maybe _after_ the patchset is in?
That's fine with me. I have one more question about this patch series
though: if you mention 64-bit LUNs, are you referring to multi-level
LUNs only or also to so-called extended LUNs ? I think the byte
reordering done by scsilun_to_int() is fine for multi-level LUNs but
unnatural for extended LUNs. As an example, in SAM-5 the format for
eight byte extended LUNs is defined as follows (paragraph 4.7.7.5.3):
byte 0: address method (3), length (2) and extended address method (2)
bytes 1..7: long extended flat space LUN with the MSB in byte 1 and LSB
in byte 7.
Today scsilun_to_int() does not preserve the MSB..LSB byte order for
extended LUNs. I think if we want to preserve the byte order in
scsilun_to_int() that that function will have to be made dependent on
the LUN addressing method. That would make scsilun_to_int() more
complex. Hence the proposal to cache the SCSI LUN such that the
queuecommand() functions are not slowed down by a call into
int_to_scsilun().
The only constant with LUNs is that T10 will keep tinkering with
them. The original LUNs were 3 bits long embedded in the cdb.
Today we have 65 bit LUNs (the 64 you have been looking at
and LU_CONG in the INQUIRY response). Decoding LUNs is best
avoided if possible.
int_to_scsilun() was a hack that made 16 bit LUNs look half
reasonable as integers. Beyond 16 bits, it just looks like
a random number generator. As long as the "int" that is
produced can be mapped to a T10 LUN and vice versa, it doesn't
matter.
Doug Gilbert
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
