[SCSI] don't change sdev starvation list order without request dispatched

The sdev is deleted from starved list and then try to dispatch from this
device. It's quite possible the sdev can't eventually dispatch a request,
then the sdev will be in starved list tail. This isn't fair.
There are two cases here:
1. unplug path. scsi_request_fn() calls to scsi_target_queue_ready(), then
the dev is removed from starved list, but quite possible host queue isn't
ready, the dev is moved to starved list without dispatching any request.
2. scsi_run_queue path. It deletes the dev from starved list first (both
global and local starved lists), then handles the dev. Then we could have
the same process like case 1.

This patch fixes the first case. Case 2 isn't fixed, because there is a
rare case scsi_run_queue finds host isn't busy but scsi_request_fn finds
host is busy (other CPU is faster to get host queue depth). Not deleting
the dev from starved list in scsi_run_queue will keep scsi_run_queue
looping (though this is very rare case, because host will become busy).
Fortunately fixing case 1 already gives big improvement for starvation in
my test. In a 12 disk JBOD setup, running file creation under EXT4, this
gives 12% more throughput.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f85cfa6c47b5212982bcbfcd9065936c27360f3d..b2c95dbe9d651d3a82b9e7a269ec3b5bd7e7c167 100644 (file)
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1316,15 +1316,10 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
        }
 
        if (scsi_target_is_busy(starget)) {
-               if (list_empty(&sdev->starved_entry))
-                       list_add_tail(&sdev->starved_entry,
-                                     &shost->starved_list);
+               list_move_tail(&sdev->starved_entry, &shost->starved_list);
                return 0;
        }
 
-       /* We're OK to process the command, so we can't be starved */
-       if (!list_empty(&sdev->starved_entry))
-               list_del_init(&sdev->starved_entry);
        return 1;
 }