* [PATCH] qla4xx: a small loop fix
@ 2011-11-01 16:46 Tomas Henzl
2011-11-01 18:10 ` Mike Christie
0 siblings, 1 reply; 4+ messages in thread
From: Tomas Henzl @ 2011-11-01 16:46 UTC (permalink / raw)
To: 'linux-scsi@vger.kernel.org'
Cc: vikas.chaudhary, lalit.chandivade, ravi.anand
From: Tomas Henzl <thenzl@redhat.com>
When the qla4xxx_get_fwddb_entry returns QLA_ERROR
the nex_idx is not updated,
for (idx = 0; idx < max_ddbs; idx = next_idx) {
ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
&next_idx, &state, &conn_err,
NULL, NULL);
if (ret == QLA_ERROR)
continue;
This means there is a risk that the 'idx < max_ddbs' condition will never
met and the loop will loop forever.
Fix this by explicitly increasing the next_idx in the error condition.
Maybe a break instead of continue is more appropriate, leaving the decision
on the qlogic maintainer.
Signed-off-by: Tomas Henzl <thenzl@redhat.com>
diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
index 3075fba..17acb17 100644
--- a/drivers/scsi/qla4xxx/ql4_init.c
+++ b/drivers/scsi/qla4xxx/ql4_init.c
@@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
&next_idx, &state, &conn_err,
NULL, NULL);
- if (ret == QLA_ERROR)
+ if (ret == QLA_ERROR) {
+ next_idx++;
continue;
+ }
if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
state == DDB_DS_SESSION_FAILED) {
DEBUG2(ql4_printk(KERN_INFO, ha,
--
1.7.6.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] qla4xx: a small loop fix
2011-11-01 16:46 [PATCH] qla4xx: a small loop fix Tomas Henzl
@ 2011-11-01 18:10 ` Mike Christie
2011-11-01 20:22 ` Tomas Henzl
0 siblings, 1 reply; 4+ messages in thread
From: Mike Christie @ 2011-11-01 18:10 UTC (permalink / raw)
To: Tomas Henzl
Cc: 'linux-scsi@vger.kernel.org', vikas.chaudhary,
lalit.chandivade, ravi.anand
On 11/01/2011 11:46 AM, Tomas Henzl wrote:
> From: Tomas Henzl <thenzl@redhat.com>
>
> When the qla4xxx_get_fwddb_entry returns QLA_ERROR
> the nex_idx is not updated,
> for (idx = 0; idx < max_ddbs; idx = next_idx) {
> ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
> &next_idx, &state, &conn_err,
> NULL, NULL);
> if (ret == QLA_ERROR)
> continue;
>
> This means there is a risk that the 'idx < max_ddbs' condition will never
> met and the loop will loop forever.
> Fix this by explicitly increasing the next_idx in the error condition.
>
> Maybe a break instead of continue is more appropriate, leaving the decision
> on the qlogic maintainer.
>
> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
>
> diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
> index 3075fba..17acb17 100644
> --- a/drivers/scsi/qla4xxx/ql4_init.c
> +++ b/drivers/scsi/qla4xxx/ql4_init.c
> @@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
> ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
> &next_idx, &state, &conn_err,
> NULL, NULL);
> - if (ret == QLA_ERROR)
> + if (ret == QLA_ERROR) {
> + next_idx++;
> continue;
> + }
> if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
> state == DDB_DS_SESSION_FAILED) {
> DEBUG2(ql4_printk(KERN_INFO, ha,
Patch looks correct.
James, this is a patch for something that is not yet upstream. I will
just merge this patch with the patch I was going to send upstream, so do
not worry about it.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] qla4xx: a small loop fix
2011-11-01 18:10 ` Mike Christie
@ 2011-11-01 20:22 ` Tomas Henzl
2011-11-01 20:31 ` Mike Christie
0 siblings, 1 reply; 4+ messages in thread
From: Tomas Henzl @ 2011-11-01 20:22 UTC (permalink / raw)
To: Mike Christie
Cc: 'linux-scsi@vger.kernel.org', vikas.chaudhary,
lalit.chandivade, ravi.anand
On 11/01/2011 07:10 PM, Mike Christie wrote:
> On 11/01/2011 11:46 AM, Tomas Henzl wrote:
>> From: Tomas Henzl <thenzl@redhat.com>
>>
>> When the qla4xxx_get_fwddb_entry returns QLA_ERROR
>> the nex_idx is not updated,
>> for (idx = 0; idx < max_ddbs; idx = next_idx) {
>> ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
>> &next_idx, &state, &conn_err,
>> NULL, NULL);
>> if (ret == QLA_ERROR)
>> continue;
>>
>> This means there is a risk that the 'idx < max_ddbs' condition will never
>> met and the loop will loop forever.
>> Fix this by explicitly increasing the next_idx in the error condition.
>>
>> Maybe a break instead of continue is more appropriate, leaving the decision
>> on the qlogic maintainer.
>>
>> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
>>
>> diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
>> index 3075fba..17acb17 100644
>> --- a/drivers/scsi/qla4xxx/ql4_init.c
>> +++ b/drivers/scsi/qla4xxx/ql4_init.c
>> @@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
>> ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
>> &next_idx, &state, &conn_err,
>> NULL, NULL);
>> - if (ret == QLA_ERROR)
>> + if (ret == QLA_ERROR) {
>> + next_idx++;
>> continue;
>> + }
>> if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
>> state == DDB_DS_SESSION_FAILED) {
>> DEBUG2(ql4_printk(KERN_INFO, ha,
> Patch looks correct.
>
> James, this is a patch for something that is not yet upstream. I will
> just merge this patch with the patch I was going to send upstream, so do
> not worry about it.
It's created on top of scsi-misc (99a700bcc75429ba84a672d04f0b650dcc5b3042 [SCSI] mv_sas: OCZ RevoDrive3 & zDrive R4 support)
it looks like being created on top of the not yet posted patch, but it isn't so :)
To the method how this gets accepted i don't care.
> --
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] qla4xx: a small loop fix
2011-11-01 20:22 ` Tomas Henzl
@ 2011-11-01 20:31 ` Mike Christie
0 siblings, 0 replies; 4+ messages in thread
From: Mike Christie @ 2011-11-01 20:31 UTC (permalink / raw)
To: Tomas Henzl
Cc: 'linux-scsi@vger.kernel.org', vikas.chaudhary,
lalit.chandivade, ravi.anand
On 11/01/2011 03:22 PM, Tomas Henzl wrote:
> On 11/01/2011 07:10 PM, Mike Christie wrote:
>> On 11/01/2011 11:46 AM, Tomas Henzl wrote:
>>> From: Tomas Henzl <thenzl@redhat.com>
>>>
>>> When the qla4xxx_get_fwddb_entry returns QLA_ERROR
>>> the nex_idx is not updated,
>>> for (idx = 0; idx < max_ddbs; idx = next_idx) {
>>> ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
>>> &next_idx, &state, &conn_err,
>>> NULL, NULL);
>>> if (ret == QLA_ERROR)
>>> continue;
>>>
>>> This means there is a risk that the 'idx < max_ddbs' condition will never
>>> met and the loop will loop forever.
>>> Fix this by explicitly increasing the next_idx in the error condition.
>>>
>>> Maybe a break instead of continue is more appropriate, leaving the decision
>>> on the qlogic maintainer.
>>>
>>> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
>>>
>>> diff --git a/drivers/scsi/qla4xxx/ql4_init.c b/drivers/scsi/qla4xxx/ql4_init.c
>>> index 3075fba..17acb17 100644
>>> --- a/drivers/scsi/qla4xxx/ql4_init.c
>>> +++ b/drivers/scsi/qla4xxx/ql4_init.c
>>> @@ -787,8 +787,10 @@ static void qla4xxx_free_ddb_index(struct scsi_qla_host *ha)
>>> ret = qla4xxx_get_fwddb_entry(ha, idx, NULL, 0, NULL,
>>> &next_idx, &state, &conn_err,
>>> NULL, NULL);
>>> - if (ret == QLA_ERROR)
>>> + if (ret == QLA_ERROR) {
>>> + next_idx++;
>>> continue;
>>> + }
>>> if (state == DDB_DS_NO_CONNECTION_ACTIVE ||
>>> state == DDB_DS_SESSION_FAILED) {
>>> DEBUG2(ql4_printk(KERN_INFO, ha,
>> Patch looks correct.
>>
>> James, this is a patch for something that is not yet upstream. I will
>> just merge this patch with the patch I was going to send upstream, so do
>> not worry about it.
> It's created on top of scsi-misc (99a700bcc75429ba84a672d04f0b650dcc5b3042 [SCSI] mv_sas: OCZ RevoDrive3 & zDrive R4 support)
> it looks like being created on top of the not yet posted patch, but it isn't so :)
> To the method how this gets accepted i don't care.
>
Ah yeah, you are right. Go ahead and take it.
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-11-01 20:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-01 16:46 [PATCH] qla4xx: a small loop fix Tomas Henzl
2011-11-01 18:10 ` Mike Christie
2011-11-01 20:22 ` Tomas Henzl
2011-11-01 20:31 ` Mike Christie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox