Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 11/05/2015 02:26 AM, Laurent Vivier wrote: > > > On 05/11/2015 00:06, Brian King wrote: >> On 11/04/2015 07:02 AM, Hannes Reinecke wrote: >>> On 11/04/2015 12:46 PM, Laurent Vivier wrote: On 04/11/2015 12:16, Hannes Reinecke wrote: > On 11/04/2015 11:20 AM, Laurent Vivier wrote: >> QEMU allows until 32 LUNs. >> >> Signed-off-by: Laurent Vivier >> --- >> drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c >> b/drivers/scsi/ibmvscsi/ibmvscsi.c >> index 04de287..4480d3e 100644 >> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c >> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c >> @@ -84,6 +84,7 @@ >> */ >> static int max_id = 64; >> static int max_channel = 3; >> +static int max_lun = 8; >> static int init_timeout = 300; >> static int login_timeout = 60; >> static int info_timeout = 30; >> @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, >> S_IRUGO | S_IWUSR); >> MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); >> module_param_named(client_reserve, client_reserve, int, S_IRUGO ); >> MODULE_PARM_DESC(client_reserve, "Attempt client managed >> reserve/release"); >> +module_param(max_lun, int, S_IRUGO); >> +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); >> >> static void ibmvscsi_handle_crq(struct viosrp_crq *crq, >> struct ibmvscsi_host_data *hostdata); >> @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, >> const struct vio_device_id *id) >> goto init_pool_failed; >> } >> >> -host->max_lun = 8; >> +host->max_lun = max_lun; >> host->max_id = max_id; >> host->max_channel = max_channel; >> host->max_cmd_len = 16; >> > Please, don't do this. > > 'max_lun' should only be set if the HBA / transport has some hard > limitations on the number of bytes it can use. > Otherwise the scanning algorithm in scsi_scan.c should do the > correct thing, independent on the 'max_lun' setting. So you are saying we can remove the line ? >>> Ho-hum. In principle you could, as ibmvscsi is using SRP internally, >>> which does support 64 bit LUNs. >>> >>> However, due to some weird design decisions there is >>> the function 'lun_from_dev()', which mangles the incoming LUN number >>> into something ... else. >>> Which leaves only 4 bits free for the actual LUN number, requiring >>> you to use max_lun = 16. >>> >>> Personally I would just do away with that function and use the >>> incoming LUN numbers as is. >> >> I pulled out my copy of SAM, what lun_from_dev is doing is translating >> the incoming bus / id / LUN to a LUN in the logical unit addressing format, >> as defined in 4.6.9 of SAM-4. >> >> -- >> |Bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | >> -- >> | n | (10b) | Target | >> -- >> | n+1| Bus |LUN| >> -- >> >> So this means, in the current implementation, we have 6 bits for target >> (max=63), >> 3 bits for bus (max=7), and 5 bits for LUN (max=31). >> >> It might not be a bad idea to enforce these limits on the module parameters. >> Otherwise >> we'll have a mess when we run through lun_from_dev... >> >> As far as eliminating lun_from_dev and just passing the LUN through, I don't >> think we >> can do that. The LUN list returned by Report LUNs to a VSCSI disk assumes we >> are >> doing the translation above. > > So what I understand is this patch is OK but I have to check the given > value is less than 32 (the same max value as for QEMU) ? As far as I'm concerned, yes. Thanks, Brian -- Brian King Power Linux I/O IBM Linux Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 11/05/2015 02:51 AM, Hannes Reinecke wrote: > On 11/05/2015 12:06 AM, Brian King wrote: >> On 11/04/2015 07:02 AM, Hannes Reinecke wrote: >>> On 11/04/2015 12:46 PM, Laurent Vivier wrote: On 04/11/2015 12:16, Hannes Reinecke wrote: > On 11/04/2015 11:20 AM, Laurent Vivier wrote: >> QEMU allows until 32 LUNs. >> >> Signed-off-by: Laurent Vivier >> --- >> drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c >> b/drivers/scsi/ibmvscsi/ibmvscsi.c >> index 04de287..4480d3e 100644 >> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c >> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c >> @@ -84,6 +84,7 @@ >> */ >> static int max_id = 64; >> static int max_channel = 3; >> +static int max_lun = 8; >> static int init_timeout = 300; >> static int login_timeout = 60; >> static int info_timeout = 30; >> @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, >> S_IRUGO | S_IWUSR); >> MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); >> module_param_named(client_reserve, client_reserve, int, S_IRUGO ); >> MODULE_PARM_DESC(client_reserve, "Attempt client managed >> reserve/release"); >> +module_param(max_lun, int, S_IRUGO); >> +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); >> >> static void ibmvscsi_handle_crq(struct viosrp_crq *crq, >> struct ibmvscsi_host_data *hostdata); >> @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, >> const struct vio_device_id *id) >> goto init_pool_failed; >> } >> >> -host->max_lun = 8; >> +host->max_lun = max_lun; >> host->max_id = max_id; >> host->max_channel = max_channel; >> host->max_cmd_len = 16; >> > Please, don't do this. > > 'max_lun' should only be set if the HBA / transport has some hard > limitations on the number of bytes it can use. > Otherwise the scanning algorithm in scsi_scan.c should do the > correct thing, independent on the 'max_lun' setting. So you are saying we can remove the line ? >>> Ho-hum. In principle you could, as ibmvscsi is using SRP internally, >>> which does support 64 bit LUNs. >>> >>> However, due to some weird design decisions there is >>> the function 'lun_from_dev()', which mangles the incoming LUN number >>> into something ... else. >>> Which leaves only 4 bits free for the actual LUN number, requiring >>> you to use max_lun = 16. >>> >>> Personally I would just do away with that function and use the >>> incoming LUN numbers as is. >> >> I pulled out my copy of SAM, what lun_from_dev is doing is translating >> the incoming bus / id / LUN to a LUN in the logical unit addressing format, >> as defined in 4.6.9 of SAM-4. >> >> -- >> |Bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | >> -- >> | n | (10b) | Target | >> -- >> | n+1| Bus |LUN| >> -- >> >> So this means, in the current implementation, we have 6 bits for target >> (max=63), >> 3 bits for bus (max=7), and 5 bits for LUN (max=31). >> >> It might not be a bad idea to enforce these limits on the module parameters. >> Otherwise >> we'll have a mess when we run through lun_from_dev... >> > Correct. With the current implementation we need to set max_lun to 31. > >> As far as eliminating lun_from_dev and just passing the LUN through, I don't >> think we >> can do that. The LUN list returned by Report LUNs to a VSCSI disk assumes we >> are >> doing the translation above. >> > ? How can that be ? I was just going by what sg_luns returned on a VSCSI disk: [root@powerio-blue-lp6 ~]# lsscsi [0:0:1:0]diskAIX VDASD0001 /dev/sda [0:0:2:0]cd/dvd AIX VOPTA /dev/sr0 [0:0:3:0]diskAIX VDASD0001 /dev/sdb [0:0:4:0]diskAIX VDASD0001 /dev/sdc [0:0:7:0]cd/dvd AIX VOPTA /dev/sr1 [root@powerio-blue-lp6 ~]# sg_luns /dev/sda Lun list length = 8 which imples 1 lun entry Report luns [select_report=0x0]: [root@powerio-blue-lp6 ~]# sg_luns /dev/sdb Lun list length = 8 which imples 1 lun entry Report luns [select_report=0x0]: This is without your patch, but I don't think there should be any filtering of report luns even in this case. Its what the VIOS is returning that is at issue here. Thanks, Brian -- Brian King Power Linux I/O IBM Linux Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 11/05/2015 12:06 AM, Brian King wrote: > On 11/04/2015 07:02 AM, Hannes Reinecke wrote: >> On 11/04/2015 12:46 PM, Laurent Vivier wrote: >>> >>> >>> On 04/11/2015 12:16, Hannes Reinecke wrote: On 11/04/2015 11:20 AM, Laurent Vivier wrote: > QEMU allows until 32 LUNs. > > Signed-off-by: Laurent Vivier > --- > drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c > b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 04de287..4480d3e 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -84,6 +84,7 @@ > */ > static int max_id = 64; > static int max_channel = 3; > +static int max_lun = 8; > static int init_timeout = 300; > static int login_timeout = 60; > static int info_timeout = 30; > @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO > | S_IWUSR); > MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); > module_param_named(client_reserve, client_reserve, int, S_IRUGO ); > MODULE_PARM_DESC(client_reserve, "Attempt client managed > reserve/release"); > +module_param(max_lun, int, S_IRUGO); > +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); > > static void ibmvscsi_handle_crq(struct viosrp_crq *crq, > struct ibmvscsi_host_data *hostdata); > @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, > const struct vio_device_id *id) > goto init_pool_failed; > } > > - host->max_lun = 8; > + host->max_lun = max_lun; > host->max_id = max_id; > host->max_channel = max_channel; > host->max_cmd_len = 16; > Please, don't do this. 'max_lun' should only be set if the HBA / transport has some hard limitations on the number of bytes it can use. Otherwise the scanning algorithm in scsi_scan.c should do the correct thing, independent on the 'max_lun' setting. >>> >>> So you are saying we can remove the line ? >>> >> Ho-hum. In principle you could, as ibmvscsi is using SRP internally, >> which does support 64 bit LUNs. >> >> However, due to some weird design decisions there is >> the function 'lun_from_dev()', which mangles the incoming LUN number >> into something ... else. >> Which leaves only 4 bits free for the actual LUN number, requiring >> you to use max_lun = 16. >> >> Personally I would just do away with that function and use the >> incoming LUN numbers as is. > > I pulled out my copy of SAM, what lun_from_dev is doing is translating > the incoming bus / id / LUN to a LUN in the logical unit addressing format, > as defined in 4.6.9 of SAM-4. > > -- > |Bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | > -- > | n | (10b) | Target | > -- > | n+1| Bus |LUN| > -- > > So this means, in the current implementation, we have 6 bits for target > (max=63), > 3 bits for bus (max=7), and 5 bits for LUN (max=31). > > It might not be a bad idea to enforce these limits on the module parameters. > Otherwise > we'll have a mess when we run through lun_from_dev... > Correct. With the current implementation we need to set max_lun to 31. > As far as eliminating lun_from_dev and just passing the LUN through, I don't > think we > can do that. The LUN list returned by Report LUNs to a VSCSI disk assumes we > are > doing the translation above. > ? How can that be ? REPORT LUNs will give you a list of available LUNs _from that target_. As the SCSI stack now moved to handle full 64bit LUNs the values from REPORT LUNs will be used as-is (well, byte-swapped, but still). Can you test with the attached patch? Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) From 42d920e9ff2a4739d4a901e033a93364586c840c Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Thu, 5 Nov 2015 09:50:23 +0100 Subject: [PATCH] ibmvscsi: Allow 64bit LUNs The SRP protocol allows full 64bit LUNs, so there is no need to mangle the LUNs in the driver. Signed-off-by: Hannes Reinecke --- drivers/scsi/ibmvscsi/ibmvscsi.c | 25 ++--- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 6a41c36..0162b1d 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -1019,16 +1019,6 @@ static void handle_cmd_rsp(struct srp_event_struct *evt_struct) } /** - * lun_from_dev: - Returns the lun of the sc
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 05/11/2015 00:06, Brian King wrote: > On 11/04/2015 07:02 AM, Hannes Reinecke wrote: >> On 11/04/2015 12:46 PM, Laurent Vivier wrote: >>> >>> >>> On 04/11/2015 12:16, Hannes Reinecke wrote: On 11/04/2015 11:20 AM, Laurent Vivier wrote: > QEMU allows until 32 LUNs. > > Signed-off-by: Laurent Vivier > --- > drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c > b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 04de287..4480d3e 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -84,6 +84,7 @@ > */ > static int max_id = 64; > static int max_channel = 3; > +static int max_lun = 8; > static int init_timeout = 300; > static int login_timeout = 60; > static int info_timeout = 30; > @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO > | S_IWUSR); > MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); > module_param_named(client_reserve, client_reserve, int, S_IRUGO ); > MODULE_PARM_DESC(client_reserve, "Attempt client managed > reserve/release"); > +module_param(max_lun, int, S_IRUGO); > +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); > > static void ibmvscsi_handle_crq(struct viosrp_crq *crq, > struct ibmvscsi_host_data *hostdata); > @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, > const struct vio_device_id *id) > goto init_pool_failed; > } > > - host->max_lun = 8; > + host->max_lun = max_lun; > host->max_id = max_id; > host->max_channel = max_channel; > host->max_cmd_len = 16; > Please, don't do this. 'max_lun' should only be set if the HBA / transport has some hard limitations on the number of bytes it can use. Otherwise the scanning algorithm in scsi_scan.c should do the correct thing, independent on the 'max_lun' setting. >>> >>> So you are saying we can remove the line ? >>> >> Ho-hum. In principle you could, as ibmvscsi is using SRP internally, >> which does support 64 bit LUNs. >> >> However, due to some weird design decisions there is >> the function 'lun_from_dev()', which mangles the incoming LUN number >> into something ... else. >> Which leaves only 4 bits free for the actual LUN number, requiring >> you to use max_lun = 16. >> >> Personally I would just do away with that function and use the >> incoming LUN numbers as is. > > I pulled out my copy of SAM, what lun_from_dev is doing is translating > the incoming bus / id / LUN to a LUN in the logical unit addressing format, > as defined in 4.6.9 of SAM-4. > > -- > |Bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | > -- > | n | (10b) | Target | > -- > | n+1| Bus |LUN| > -- > > So this means, in the current implementation, we have 6 bits for target > (max=63), > 3 bits for bus (max=7), and 5 bits for LUN (max=31). > > It might not be a bad idea to enforce these limits on the module parameters. > Otherwise > we'll have a mess when we run through lun_from_dev... > > As far as eliminating lun_from_dev and just passing the LUN through, I don't > think we > can do that. The LUN list returned by Report LUNs to a VSCSI disk assumes we > are > doing the translation above. So what I understand is this patch is OK but I have to check the given value is less than 32 (the same max value as for QEMU) ? Laurent -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 11/04/2015 07:02 AM, Hannes Reinecke wrote: > On 11/04/2015 12:46 PM, Laurent Vivier wrote: >> >> >> On 04/11/2015 12:16, Hannes Reinecke wrote: >>> On 11/04/2015 11:20 AM, Laurent Vivier wrote: QEMU allows until 32 LUNs. Signed-off-by: Laurent Vivier --- drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 04de287..4480d3e 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -84,6 +84,7 @@ */ static int max_id = 64; static int max_channel = 3; +static int max_lun = 8; static int init_timeout = 300; static int login_timeout = 60; static int info_timeout = 30; @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); module_param_named(client_reserve, client_reserve, int, S_IRUGO ); MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); +module_param(max_lun, int, S_IRUGO); +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); static void ibmvscsi_handle_crq(struct viosrp_crq *crq, struct ibmvscsi_host_data *hostdata); @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id) goto init_pool_failed; } - host->max_lun = 8; + host->max_lun = max_lun; host->max_id = max_id; host->max_channel = max_channel; host->max_cmd_len = 16; >>> Please, don't do this. >>> >>> 'max_lun' should only be set if the HBA / transport has some hard >>> limitations on the number of bytes it can use. >>> Otherwise the scanning algorithm in scsi_scan.c should do the >>> correct thing, independent on the 'max_lun' setting. >> >> So you are saying we can remove the line ? >> > Ho-hum. In principle you could, as ibmvscsi is using SRP internally, > which does support 64 bit LUNs. > > However, due to some weird design decisions there is > the function 'lun_from_dev()', which mangles the incoming LUN number > into something ... else. > Which leaves only 4 bits free for the actual LUN number, requiring > you to use max_lun = 16. > > Personally I would just do away with that function and use the > incoming LUN numbers as is. I pulled out my copy of SAM, what lun_from_dev is doing is translating the incoming bus / id / LUN to a LUN in the logical unit addressing format, as defined in 4.6.9 of SAM-4. -- |Bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | -- | n | (10b) | Target | -- | n+1| Bus |LUN| -- So this means, in the current implementation, we have 6 bits for target (max=63), 3 bits for bus (max=7), and 5 bits for LUN (max=31). It might not be a bad idea to enforce these limits on the module parameters. Otherwise we'll have a mess when we run through lun_from_dev... As far as eliminating lun_from_dev and just passing the LUN through, I don't think we can do that. The LUN list returned by Report LUNs to a VSCSI disk assumes we are doing the translation above. -Brian -- Brian King Power Linux I/O IBM Linux Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 11/04/2015 12:46 PM, Laurent Vivier wrote: > > > On 04/11/2015 12:16, Hannes Reinecke wrote: >> On 11/04/2015 11:20 AM, Laurent Vivier wrote: >>> QEMU allows until 32 LUNs. >>> >>> Signed-off-by: Laurent Vivier >>> --- >>> drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c >>> b/drivers/scsi/ibmvscsi/ibmvscsi.c >>> index 04de287..4480d3e 100644 >>> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c >>> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c >>> @@ -84,6 +84,7 @@ >>> */ >>> static int max_id = 64; >>> static int max_channel = 3; >>> +static int max_lun = 8; >>> static int init_timeout = 300; >>> static int login_timeout = 60; >>> static int info_timeout = 30; >>> @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | >>> S_IWUSR); >>> MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); >>> module_param_named(client_reserve, client_reserve, int, S_IRUGO ); >>> MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); >>> +module_param(max_lun, int, S_IRUGO); >>> +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); >>> >>> static void ibmvscsi_handle_crq(struct viosrp_crq *crq, >>> struct ibmvscsi_host_data *hostdata); >>> @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const >>> struct vio_device_id *id) >>> goto init_pool_failed; >>> } >>> >>> - host->max_lun = 8; >>> + host->max_lun = max_lun; >>> host->max_id = max_id; >>> host->max_channel = max_channel; >>> host->max_cmd_len = 16; >>> >> Please, don't do this. >> >> 'max_lun' should only be set if the HBA / transport has some hard >> limitations on the number of bytes it can use. >> Otherwise the scanning algorithm in scsi_scan.c should do the >> correct thing, independent on the 'max_lun' setting. > > So you are saying we can remove the line ? > Ho-hum. In principle you could, as ibmvscsi is using SRP internally, which does support 64 bit LUNs. However, due to some weird design decisions there is the function 'lun_from_dev()', which mangles the incoming LUN number into something ... else. Which leaves only 4 bits free for the actual LUN number, requiring you to use max_lun = 16. Personally I would just do away with that function and use the incoming LUN numbers as is. Brian? Any comments? Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 04/11/2015 12:16, Hannes Reinecke wrote: > On 11/04/2015 11:20 AM, Laurent Vivier wrote: >> QEMU allows until 32 LUNs. >> >> Signed-off-by: Laurent Vivier >> --- >> drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c >> b/drivers/scsi/ibmvscsi/ibmvscsi.c >> index 04de287..4480d3e 100644 >> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c >> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c >> @@ -84,6 +84,7 @@ >> */ >> static int max_id = 64; >> static int max_channel = 3; >> +static int max_lun = 8; >> static int init_timeout = 300; >> static int login_timeout = 60; >> static int info_timeout = 30; >> @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | >> S_IWUSR); >> MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); >> module_param_named(client_reserve, client_reserve, int, S_IRUGO ); >> MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); >> +module_param(max_lun, int, S_IRUGO); >> +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); >> >> static void ibmvscsi_handle_crq(struct viosrp_crq *crq, >> struct ibmvscsi_host_data *hostdata); >> @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const >> struct vio_device_id *id) >> goto init_pool_failed; >> } >> >> -host->max_lun = 8; >> +host->max_lun = max_lun; >> host->max_id = max_id; >> host->max_channel = max_channel; >> host->max_cmd_len = 16; >> > Please, don't do this. > > 'max_lun' should only be set if the HBA / transport has some hard > limitations on the number of bytes it can use. > Otherwise the scanning algorithm in scsi_scan.c should do the > correct thing, independent on the 'max_lun' setting. So you are saying we can remove the line ? > If qemu has some issues here someone should rather fix qemu ... There is no issue with QEMU. QEMU can manage more than "8" LUNs, and we'd like to. Laurent -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 11/04/2015 11:20 AM, Laurent Vivier wrote: > QEMU allows until 32 LUNs. > > Signed-off-by: Laurent Vivier > --- > drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c > b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 04de287..4480d3e 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -84,6 +84,7 @@ > */ > static int max_id = 64; > static int max_channel = 3; > +static int max_lun = 8; > static int init_timeout = 300; > static int login_timeout = 60; > static int info_timeout = 30; > @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | > S_IWUSR); > MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); > module_param_named(client_reserve, client_reserve, int, S_IRUGO ); > MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); > +module_param(max_lun, int, S_IRUGO); > +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); > > static void ibmvscsi_handle_crq(struct viosrp_crq *crq, > struct ibmvscsi_host_data *hostdata); > @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const > struct vio_device_id *id) > goto init_pool_failed; > } > > - host->max_lun = 8; > + host->max_lun = max_lun; > host->max_id = max_id; > host->max_channel = max_channel; > host->max_cmd_len = 16; > Please, don't do this. 'max_lun' should only be set if the HBA / transport has some hard limitations on the number of bytes it can use. Otherwise the scanning algorithm in scsi_scan.c should do the correct thing, independent on the 'max_lun' setting. If qemu has some issues here someone should rather fix qemu ... Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
On 04/11/2015 11:20, Laurent Vivier wrote: > QEMU allows until 32 LUNs. > > Signed-off-by: Laurent Vivier I forget the: Reviewed-by: Brian King Acked-by: Tyrel Datwyler > --- > drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c > b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 04de287..4480d3e 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -84,6 +84,7 @@ > */ > static int max_id = 64; > static int max_channel = 3; > +static int max_lun = 8; > static int init_timeout = 300; > static int login_timeout = 60; > static int info_timeout = 30; > @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | > S_IWUSR); > MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); > module_param_named(client_reserve, client_reserve, int, S_IRUGO ); > MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); > +module_param(max_lun, int, S_IRUGO); > +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); > > static void ibmvscsi_handle_crq(struct viosrp_crq *crq, > struct ibmvscsi_host_data *hostdata); > @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const > struct vio_device_id *id) > goto init_pool_failed; > } > > - host->max_lun = 8; > + host->max_lun = max_lun; > host->max_id = max_id; > host->max_channel = max_channel; > host->max_cmd_len = 16; > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] ibmvscsi: Allow to configure maximum LUN
QEMU allows until 32 LUNs. Signed-off-by: Laurent Vivier --- drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 04de287..4480d3e 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -84,6 +84,7 @@ */ static int max_id = 64; static int max_channel = 3; +static int max_lun = 8; static int init_timeout = 300; static int login_timeout = 60; static int info_timeout = 30; @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); module_param_named(client_reserve, client_reserve, int, S_IRUGO ); MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); +module_param(max_lun, int, S_IRUGO); +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); static void ibmvscsi_handle_crq(struct viosrp_crq *crq, struct ibmvscsi_host_data *hostdata); @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id) goto init_pool_failed; } - host->max_lun = 8; + host->max_lun = max_lun; host->max_id = max_id; host->max_channel = max_channel; host->max_cmd_len = 16; -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
