Re: [PATCH] SCSI: fix the return value of scsi_target_queue_read()

Re: [PATCH] SCSI: fix the return value of scsi_target_queue_read()

[Mike Christie]

On 09/14/2010 08:35 AM, Hillf Danton wrote:
> It seems that zero should be returned if scsi_target_is_busy(starget) is
> true, no matter if sdev is on the starved list.
>
> Signed-off-by: Hillf Danton<dhillf@gmail.com>
> ---
>
> --- o/linux-2.6.36-rc1/drivers/scsi/scsi_lib.c 2010-08-16 08:41:38.000000000
> +0800
> +++ m/linux-2.6.36-rc1/drivers/scsi/scsi_lib.c 2010-09-14 21:04:50.000000000
> +0800
> @@ -1279,8 +1279,8 @@ static inline int scsi_target_queue_read
>    if (list_empty(&sdev->starved_entry)) {
>    list_add_tail(&sdev->starved_entry,
>         &shost->starved_list);
> - return 0;
>    }

I think the formatting in the patch got messed up by your mailer. When 
you resend the patch, you can remove the brackets since you removed the 
return 0 there is no need for them.

> + return 0;
>    }
>
>    /* We're OK to process the command, so we can't be starved */
>

Patch is right. I do not know what I was thinking when I did that.

Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>

Re: [PATCH] SCSI: fix the return value of scsi_target_queue_read()

[Hillf Danton]

the recommended version in plain text
---

--- o/linux-2.6.36-rc1/drivers/scsi/scsi_lib.c	2010-08-16
08:41:38.000000000 +0800
+++ m/linux-2.6.36-rc1/drivers/scsi/scsi_lib.c	2010-09-18
08:59:50.000000000 +0800
@@ -1276,11 +1276,10 @@ static inline int scsi_target_queue_read
 	}

 	if (scsi_target_is_busy(starget)) {
-		if (list_empty(&sdev->starved_entry)) {
+		if (list_empty(&sdev->starved_entry))
 			list_add_tail(&sdev->starved_entry,
 				      &shost->starved_list);
-			return 0;
-		}
+		return 0;
 	}

 	/* We're OK to process the command, so we can't be starved */




On Wed, Sep 15, 2010 at 3:15 AM, Mike Christie <michaelc@cs.wisc.edu> wrote:
> On 09/14/2010 08:35 AM, Hillf Danton wrote:
>>
>> It seems that zero should be returned if scsi_target_is_busy(starget) is
>> true, no matter if sdev is on the starved list.
>>
>> Signed-off-by: Hillf Danton<dhillf@gmail.com>
>> ---
>>
>> --- o/linux-2.6.36-rc1/drivers/scsi/scsi_lib.c 2010-08-16
>> 08:41:38.000000000
>> +0800
>> +++ m/linux-2.6.36-rc1/drivers/scsi/scsi_lib.c 2010-09-14
>> 21:04:50.000000000
>> +0800
>> @@ -1279,8 +1279,8 @@ static inline int scsi_target_queue_read
>>   if (list_empty(&sdev->starved_entry)) {
>>   list_add_tail(&sdev->starved_entry,
>>        &shost->starved_list);
>> - return 0;
>>   }
>
> I think the formatting in the patch got messed up by your mailer. When you
> resend the patch, you can remove the brackets since you removed the return 0
> there is no need for them.
>
>> + return 0;
>>   }
>>
>>   /* We're OK to process the command, so we can't be starved */
>>
>
> Patch is right. I do not know what I was thinking when I did that.
>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
>
--
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

[PATCH] SCSI: fix the return value of scsi_target_queue_read()

[michaelc]

From: Hillf Danton <dhillf@gmail.com>

It seems that zero should be returned if scsi_target_is_busy(starget) is
true, no matter if sdev is on the starved list.

Signed-off-by: Hillf Danton <dhillf@gmail.com>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_lib.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5b6bbae..0ed7a66 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1278,11 +1278,10 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
 	}
 
 	if (scsi_target_is_busy(starget)) {
-		if (list_empty(&sdev->starved_entry)) {
+		if (list_empty(&sdev->starved_entry))
 			list_add_tail(&sdev->starved_entry,
 				      &shost->starved_list);
-			return 0;
-		}
+		return 0;
 	}
 
 	/* We're OK to process the command, so we can't be starved */
-- 
1.7.2.3