On Tue, Sep 21, 2010 at 1:56 AM, James Bottomley
<James.Bottomley@xxxxxxx> wrote:
> On Sun, 2010-09-19 at 20:53 +0800, Hillf Danton wrote:
>> It seems that check for host busy is deserved before scanning the starved list.
>> And list operation is simplified.
>>
>> Signed-off-by: Hillf Danton <dhillf@xxxxxxxxx>
>> ---
>>
>> --- o/linux-2.6.36-rc4/drivers/scsi/scsi_lib.c    Â2010-09-13
>> 07:07:38.000000000 +0800
>> +++ m/linux-2.6.36-rc4/drivers/scsi/scsi_lib.c    Â2010-09-19
>> 20:32:34.000000000 +0800
>> @@ -399,18 +399,20 @@ static inline int scsi_host_is_busy(stru
>> Â */
>> Âstatic void scsi_run_queue(struct request_queue *q)
>> Â{
>> - Â Â struct scsi_device *sdev = q->queuedata;
>> + Â Â struct scsi_device *sdev = q->queuedata, *safe;
>> Â Â Â struct Scsi_Host *shost = sdev->host;
>> - Â Â LIST_HEAD(starved_list);
>> Â Â Â unsigned long flags;
>>
>> Â Â Â if (scsi_target(sdev)->single_lun)
>> Â Â Â Â Â Â Â scsi_single_lun_run(sdev);
>>
>> Â Â Â spin_lock_irqsave(shost->host_lock, flags);
>> - Â Â list_splice_init(&shost->starved_list, &starved_list);
>> -
>> - Â Â while (!list_empty(&starved_list)) {
>> + Â Â if (scsi_host_is_busy(shost)) {
>> + Â Â Â Â Â Â spin_unlock_irqrestore(shost->host_lock, flags);
>> + Â Â Â Â Â Â return;
>> + Â Â }
>
> To be honest, there's no real gain from this micro-optimisation.
>
>> + Â Â list_for_each_entry_safe(sdev, safe, &shost->starved_list,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â starved_entry) {
>
> And this is definitely wrong. ÂThe list is altered in flight so the
> whole purpose of the static copy is to process only what existed before
> we began; otherwise you can get a livelock.
>
node. Then the potential livelock could be fixed by
list_for_each_entry_safe_reverse().
> James
>
--
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
