[PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[Michael Reed]

A change in lun scanning range in scsi_scan.c ( ">" becomes ">=") results
in lpfc no longer discovering lun 255 of my test configuration.  This
patch adjusts lpfc_attr.c to discover lun 255 again.

scsi: host 3 channel 0 id 17 lun255 has a LUN larger than allowed by the host adapter


Signed-off-by: Michael Reed <mdr@sgi.com>


--- scsi-misc-2.6/drivers/scsi/lpfc/lpfc_attr.c	2009-12-01 16:18:40.000000000 -0600
+++ scsi-misc-2.6-modified/drivers/scsi/lpfc/lpfc_attr.c	2009-12-04 10:01:52.091503203 -0600
@@ -3099,10 +3099,10 @@ LPFC_VPORT_ATTR(discovery_threads, 32, 1
 
 /*
 # lpfc_max_luns: maximum allowed LUN.
-# Value range is [0,65535]. Default value is 255.
+# Value range is [0,65535]. Default value is 256, 1 greater than max lun number.
 # NOTE: The SCSI layer might probe all allowed LUN on some old targets.
 */
-LPFC_VPORT_ATTR_R(max_luns, 255, 0, 65535, "Maximum allowed LUN");
+LPFC_VPORT_ATTR_R(max_luns, 256, 0, 65535, "Maximum allowed LUN");
 
 /*
 # lpfc_poll_tmo: .Milliseconds driver will wait between polling FCP ring.

Re: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[James Smart]

Thanks Mike.   Wonder if that change broke other drivers too...

Acked-By:   James Smart <james.smart@emulex.com>

-- james s

Michael Reed wrote:
> A change in lun scanning range in scsi_scan.c ( ">" becomes ">=") results
> in lpfc no longer discovering lun 255 of my test configuration.  This
> patch adjusts lpfc_attr.c to discover lun 255 again.
>
> scsi: host 3 channel 0 id 17 lun255 has a LUN larger than allowed by the host adapter
>
>
> Signed-off-by: Michael Reed <mdr@sgi.com>
>
>
> --- scsi-misc-2.6/drivers/scsi/lpfc/lpfc_attr.c	2009-12-01 16:18:40.000000000 -0600
> +++ scsi-misc-2.6-modified/drivers/scsi/lpfc/lpfc_attr.c	2009-12-04 10:01:52.091503203 -0600
> @@ -3099,10 +3099,10 @@ LPFC_VPORT_ATTR(discovery_threads, 32, 1
>  
>  /*
>  # lpfc_max_luns: maximum allowed LUN.
> -# Value range is [0,65535]. Default value is 255.
> +# Value range is [0,65535]. Default value is 256, 1 greater than max lun number.
>  # NOTE: The SCSI layer might probe all allowed LUN on some old targets.
>  */
> -LPFC_VPORT_ATTR_R(max_luns, 255, 0, 65535, "Maximum allowed LUN");
> +LPFC_VPORT_ATTR_R(max_luns, 256, 0, 65535, "Maximum allowed LUN");
>  
>  /*
>  # lpfc_poll_tmo: .Milliseconds driver will wait between polling FCP ring.
>
>   

Re: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[Michael Reed]

Yeah.  I'm working on a patch for qla1280.c also.  Probably others
have been hit.  It would have been nice if the patch author had
fixed all the drivers instead of introducing this change and waiting
for people to discover their last lun is now missing.

James Smart wrote:
> Thanks Mike.   Wonder if that change broke other drivers too...
> 
> Acked-By:   James Smart <james.smart@emulex.com>
> 
> -- james s
> 
> Michael Reed wrote:
>> A change in lun scanning range in scsi_scan.c ( ">" becomes ">=") results
>> in lpfc no longer discovering lun 255 of my test configuration.  This
>> patch adjusts lpfc_attr.c to discover lun 255 again.
>>
>> scsi: host 3 channel 0 id 17 lun255 has a LUN larger than allowed by the host adapter
>>
>>
>> Signed-off-by: Michael Reed <mdr@sgi.com>
>>
>>
>> --- scsi-misc-2.6/drivers/scsi/lpfc/lpfc_attr.c	2009-12-01 16:18:40.000000000 -0600
>> +++ scsi-misc-2.6-modified/drivers/scsi/lpfc/lpfc_attr.c	2009-12-04 10:01:52.091503203 -0600
>> @@ -3099,10 +3099,10 @@ LPFC_VPORT_ATTR(discovery_threads, 32, 1
>>  
>>  /*
>>  # lpfc_max_luns: maximum allowed LUN.
>> -# Value range is [0,65535]. Default value is 255.
>> +# Value range is [0,65535]. Default value is 256, 1 greater than max lun number.
>>  # NOTE: The SCSI layer might probe all allowed LUN on some old targets.
>>  */
>> -LPFC_VPORT_ATTR_R(max_luns, 255, 0, 65535, "Maximum allowed LUN");
>> +LPFC_VPORT_ATTR_R(max_luns, 256, 0, 65535, "Maximum allowed LUN");
>>  
>>  /*
>>  # lpfc_poll_tmo: .Milliseconds driver will wait between polling FCP ring.
>>
>>   
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[James Bottomley]

On Fri, 2009-12-04 at 11:55 -0600, Michael Reed wrote:
> Yeah.  I'm working on a patch for qla1280.c also.  Probably others
> have been hit.  It would have been nice if the patch author had
> fixed all the drivers instead of introducing this change and waiting
> for people to discover their last lun is now missing.

Actually, lets just revert this:

commit 71c309995bff5b5e84253931888b6e8163ee1df0
Author: Ed Lin <ed.lin@promise.com>
Date:   Fri Oct 9 15:23:27 2009 -0700

    [SCSI] fix inconsistent usage of max_lun

Which is causing all the problems.  Perhaps we need to update the
documentation instead?

James

Re: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[Michael Reed]

James Bottomley wrote:
> On Fri, 2009-12-04 at 11:55 -0600, Michael Reed wrote:
>> Yeah.  I'm working on a patch for qla1280.c also.  Probably others
>> have been hit.  It would have been nice if the patch author had
>> fixed all the drivers instead of introducing this change and waiting
>> for people to discover their last lun is now missing.
> 
> Actually, lets just revert this:
> 
> commit 71c309995bff5b5e84253931888b6e8163ee1df0
> Author: Ed Lin <ed.lin@promise.com>
> Date:   Fri Oct 9 15:23:27 2009 -0700
> 
>     [SCSI] fix inconsistent usage of max_lun
> 
> Which is causing all the problems.  Perhaps we need to update the
> documentation instead?
> 
> James

As much as I would whine about it, I think moving to a consistent
usage of max_lun is a good thing.  There are only about 140 references
to max_lun in the cscope output so it wouldn't be that difficult
for people to go through and fix up their drivers.  And, I suspect
a good portion will not require any change as they already either
work correctly with sequential lun scan by design or have misinterpreted
the meaning of max_lun, and thus will continue to work correctly
with report lun scan with Mr. Lin's patch applied.

I won't pretend to know the right answer.

Mike


> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[Brian King]

Michael Reed wrote:
> 
> James Bottomley wrote:
>> On Fri, 2009-12-04 at 11:55 -0600, Michael Reed wrote:
>>> Yeah.  I'm working on a patch for qla1280.c also.  Probably others
>>> have been hit.  It would have been nice if the patch author had
>>> fixed all the drivers instead of introducing this change and waiting
>>> for people to discover their last lun is now missing.
>> Actually, lets just revert this:
>>
>> commit 71c309995bff5b5e84253931888b6e8163ee1df0
>> Author: Ed Lin <ed.lin@promise.com>
>> Date:   Fri Oct 9 15:23:27 2009 -0700
>>
>>     [SCSI] fix inconsistent usage of max_lun
>>
>> Which is causing all the problems.  Perhaps we need to update the
>> documentation instead?
>>
>> James
> 
> As much as I would whine about it, I think moving to a consistent
> usage of max_lun is a good thing.  There are only about 140 references
> to max_lun in the cscope output so it wouldn't be that difficult
> for people to go through and fix up their drivers.  And, I suspect
> a good portion will not require any change as they already either
> work correctly with sequential lun scan by design or have misinterpreted
> the meaning of max_lun, and thus will continue to work correctly
> with report lun scan with Mr. Lin's patch applied.

The ibmvfc driver was using 0xffffffff to indicate it essentially had no limit
on the number of LUNs. It appears this can no longer be done with the commit
above since max_lun is an unsigned int.

-Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

Re: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[James Bottomley]

On Thu, 2009-12-10 at 07:01 -0600, Brian King wrote:
> Michael Reed wrote:
> > 
> > James Bottomley wrote:
> >> On Fri, 2009-12-04 at 11:55 -0600, Michael Reed wrote:
> >>> Yeah.  I'm working on a patch for qla1280.c also.  Probably others
> >>> have been hit.  It would have been nice if the patch author had
> >>> fixed all the drivers instead of introducing this change and waiting
> >>> for people to discover their last lun is now missing.
> >> Actually, lets just revert this:
> >>
> >> commit 71c309995bff5b5e84253931888b6e8163ee1df0
> >> Author: Ed Lin <ed.lin@promise.com>
> >> Date:   Fri Oct 9 15:23:27 2009 -0700
> >>
> >>     [SCSI] fix inconsistent usage of max_lun
> >>
> >> Which is causing all the problems.  Perhaps we need to update the
> >> documentation instead?
> >>
> >> James
> > 
> > As much as I would whine about it, I think moving to a consistent
> > usage of max_lun is a good thing.  There are only about 140 references
> > to max_lun in the cscope output so it wouldn't be that difficult
> > for people to go through and fix up their drivers.  And, I suspect
> > a good portion will not require any change as they already either
> > work correctly with sequential lun scan by design or have misinterpreted
> > the meaning of max_lun, and thus will continue to work correctly
> > with report lun scan with Mr. Lin's patch applied.
> 
> The ibmvfc driver was using 0xffffffff to indicate it essentially had no limit
> on the number of LUNs. It appears this can no longer be done with the commit
> above since max_lun is an unsigned int.

Right, I've pulled the commit.  It was largely cosmetic ... moving what
we actually do to match the documentation.  However, since it appears
everyone reads the code not the docs, the better course might be moving
the documentation to match what we actually do.

James

RE: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255

[Ed Lin - PTU]

>-----Original Message-----
>From: James Bottomley [mailto:James.Bottomley@suse.de] 
>Sent: 2009年12月10日 7:43
>To: Brian King
>Cc: Michael Reed; James Smart; linux-scsi; Ed Lin - PTU
>Subject: Re: [PATCH 1/1] lpfc: lpfc no longer discovers lun 255
>
>
>On Thu, 2009-12-10 at 07:01 -0600, Brian King wrote:
>> Michael Reed wrote:
>> > 
>> > James Bottomley wrote:
>> >> On Fri, 2009-12-04 at 11:55 -0600, Michael Reed wrote:
>> >>> Yeah.  I'm working on a patch for qla1280.c also.  
>Probably others
>> >>> have been hit.  It would have been nice if the patch author had
>> >>> fixed all the drivers instead of introducing this change 
>and waiting
>> >>> for people to discover their last lun is now missing.
>> >> Actually, lets just revert this:
>> >>
>> >> commit 71c309995bff5b5e84253931888b6e8163ee1df0
>> >> Author: Ed Lin <ed.lin@promise.com>
>> >> Date:   Fri Oct 9 15:23:27 2009 -0700
>> >>
>> >>     [SCSI] fix inconsistent usage of max_lun
>> >>
>> >> Which is causing all the problems.  Perhaps we need to update the
>> >> documentation instead?
>> >>
>> >> James
>> > 
>> > As much as I would whine about it, I think moving to a consistent
>> > usage of max_lun is a good thing.  There are only about 
>140 references
>> > to max_lun in the cscope output so it wouldn't be that difficult
>> > for people to go through and fix up their drivers.  And, I suspect
>> > a good portion will not require any change as they already either
>> > work correctly with sequential lun scan by design or have 
>misinterpreted
>> > the meaning of max_lun, and thus will continue to work correctly
>> > with report lun scan with Mr. Lin's patch applied.
>> 
>> The ibmvfc driver was using 0xffffffff to indicate it 
>essentially had no limit
>> on the number of LUNs. It appears this can no longer be done 
>with the commit
>> above since max_lun is an unsigned int.
>
>Right, I've pulled the commit.  It was largely cosmetic ... moving what
>we actually do to match the documentation.  However, since it appears
>everyone reads the code not the docs, the better course might be moving
>the documentation to match what we actually do.
>

Well, for the stex driver it is not just cosmetic. Unfortunately the member
"lun" in a request message of stex driver is only u8, which means, if user
manually scans lun 256, it will become lun 0. The original patch was intended
to address the issue.

But I didn't notice the repost lun case could have problem when preparing
the original patch. It seems there is no easy fix for it. There is no indication
which case is using sequential scan and which one is using report lun. The
confusion in the manual scan case will continue (but it may do no harm if
no one reported problem). So, I may need to prepare a patch to the stex
driver, either changing max_lun to 255 (so that user can not manually scan
lun 256), or comparing lun in the scsi command with max_lun to make sure
it is in a valid range.

Thanks,

Ed Lin